Skip to content

Commit a3cb9c8

Browse files
authored
fix(plugin-dev,runtime): retire the dev analytics stub; /analytics gates on handlerReady, not presence (#4000) (#4062)
#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.
1 parent 9e8f04d commit a3cb9c8

12 files changed

Lines changed: 240 additions & 33 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
'@objectstack/plugin-dev': patch
3+
'@objectstack/runtime': patch
4+
---
5+
6+
Retire the dev-mode `analytics` stub, and make the dispatcher gate `/analytics` on `handlerReady` rather than on service presence (#4000).
7+
8+
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.
9+
10+
- `plugin-dev` no longer registers an `analytics` dev stub; the slot stays empty (`NO_DEV_STUB_SERVICES`). Every other dev stub is unchanged.
11+
- 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.
12+
13+
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.

content/docs/kernel/services-checklist.mdx

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,13 @@ caller's `ExecutionContext` — aggregates ran without RLS/tenant scoping — an
186186
silently ignored the contract `where` filter. Without
187187
`@objectstack/service-analytics`, `/api/v1/analytics/*` now answers **404** and
188188
discovery reports `analytics: { enabled: false, status: "unavailable" }`.
189+
190+
**This holds in dev mode too** (#4000): `plugin-dev` no longer registers an
191+
`analytics` dev stub, and the dispatcher treats a slot filled by any
192+
self-declared stub (`handlerReady: false`, ADR-0076 D12) exactly like an empty
193+
one — routes unmounted, request 404. To use analytics locally, install the real
194+
engine; `@objectstack/service-analytics` runs an InMemory strategy, so it needs
195+
no database of its own.
189196
</Callout>
190197

191198
### Features (via `@objectstack/service-analytics`)

docs/adr/0076-objectql-core-tiering.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,14 +132,14 @@ Decision: each capability plugin registers its routes as a **normalized handler*
132132
**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.
133133

134134
**Inventory of current fakes / mis-reports:**
135-
- `plugin-dev` registers ~8 dev stubs — `storage` / `search` / `automation` / `graphql` / `analytics` / `realtime` / `notification` / `ai` (they already carry a `_dev: true` marker that nothing respects).
135+
- `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.)*
136136
- `ObjectQLPlugin` registers a ~66-line `analytics` **fallback** (the D10 note — deliberate, but it reports as fully available).
137137
- `http-dispatcher.ts` `svcAvailable` — the hardcode above.
138138

139139
**Decision — honest capabilities:**
140140
1. A registered service that is a stub / dev / fallback MUST self-identify with a **standard marker** (standardise one; `_dev` is the existing precedent).
141141
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.
142-
3. Consumers (agents, console) treat **only `handlerReady: true` / `status: 'available'`** as a real capability; `stub`/`unavailable` ⇒ do not use for real work.
142+
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.)*
143143

144144
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).
145145

packages/metadata-protocol/src/protocol.ts

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1408,6 +1408,19 @@ export class ObjectStackProtocolImplementation implements
14081408
data: { enabled: true, status: 'available' as const, route: '/api/v1/data', provider: 'objectql' },
14091409
};
14101410

1411+
// [#4000] The dispatcher answers a self-declared stub in the `analytics`
1412+
// slot with the same 404 an empty slot gets (`isAnalyticsServiceServeable`,
1413+
// runtime/src/domains/analytics.ts), so this builder must not advertise a
1414+
// route for it — that would be the `declared ≠ enforced` gap discovery
1415+
// exists to close. Analytics-only on purpose: every other stub-backed
1416+
// slot IS still served by its dispatcher domain, so their route
1417+
// advertisement stays presence-gated and honest. Unifying the rule
1418+
// across domains is #4058.
1419+
const analyticsUnserveable =
1420+
readServiceSelfInfo(registeredServices.get('analytics'))?.handlerReady === false;
1421+
const advertisedRoute = (serviceName: string, route?: string) =>
1422+
serviceName === 'analytics' && analyticsUnserveable ? undefined : route;
1423+
14111424
// Check which services are actually registered
14121425
for (const [serviceName, config] of Object.entries(SERVICE_CONFIG)) {
14131426
if (registeredServices.has(serviceName)) {
@@ -1420,7 +1433,7 @@ export class ObjectStackProtocolImplementation implements
14201433
services[serviceName] = {
14211434
enabled: true,
14221435
status: self?.status ?? (noHttpSurface ? ('degraded' as const) : ('available' as const)),
1423-
route: config.route,
1436+
route: advertisedRoute(serviceName, config.route),
14241437
provider: config.plugin,
14251438
...(noHttpSurface || self?.handlerReady !== undefined
14261439
? { handlerReady: noHttpSurface ? false : self?.handlerReady }
@@ -1460,10 +1473,11 @@ export class ObjectStackProtocolImplementation implements
14601473
// Add routes for available plugin services. Services without an HTTP
14611474
// surface (config.route undefined) advertise no route (D12, #2462).
14621475
for (const [serviceName, config] of Object.entries(SERVICE_CONFIG)) {
1463-
if (registeredServices.has(serviceName) && config.route) {
1476+
const route = advertisedRoute(serviceName, config.route);
1477+
if (registeredServices.has(serviceName) && route) {
14641478
const routeKey = serviceToRouteKey[serviceName];
14651479
if (routeKey) {
1466-
optionalRoutes[routeKey] = config.route;
1480+
optionalRoutes[routeKey] = route;
14671481
}
14681482
}
14691483
}

packages/objectql/src/protocol-discovery.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,25 @@ describe('ObjectStackProtocolImplementation - Dynamic Service Discovery', () =>
175175
expect(discovery.routes.analytics).toBe('/api/v1/analytics');
176176
});
177177

178+
// #4000 — the same rule one step further: a stub occupying the slot is not
179+
// the capability either. The dispatcher answers it with the empty-slot 404,
180+
// so advertising a route here would re-tell the lie #3891 removed. The
181+
// service entry itself still self-reports as a stub — that says more than
182+
// `unavailable` would.
183+
it('should not advertise the analytics route for a self-declared stub', async () => {
184+
const mockServices = new Map<string, any>();
185+
mockServices.set('analytics', { _dev: true, query: async () => ({ rows: [], fields: [] }) });
186+
187+
protocol = new ObjectStackProtocolImplementation(engine, () => mockServices);
188+
const discovery = await protocol.getDiscovery();
189+
190+
expect(discovery.services.analytics.enabled).toBe(true);
191+
expect(discovery.services.analytics.status).toBe('stub');
192+
expect(discovery.services.analytics.handlerReady).toBe(false);
193+
expect(discovery.services.analytics.route).toBeUndefined();
194+
expect(discovery.routes.analytics).toBeUndefined();
195+
});
196+
178197
it('should map file-storage service to storage route', async () => {
179198
const mockServices = new Map<string, any>();
180199
mockServices.set('file-storage', {});

packages/plugins/plugin-dev/src/dev-plugin.test.ts

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ describe('DevPlugin', () => {
6767
await expect(plugin.init(ctx)).resolves.not.toThrow();
6868
});
6969

70-
it('should register contract-compliant dev stubs for all core services', async () => {
70+
it('should register contract-compliant dev stubs for every core service except analytics', async () => {
7171
const registeredServices = new Map<string, any>();
7272
const ctx: any = {
7373
logger: {
@@ -159,12 +159,14 @@ describe('DevPlugin', () => {
159159
expect(execResult.success).toBe(true);
160160
expect(Array.isArray(await automation.listFlows())).toBe(true);
161161

162-
// ── Verify IAnalyticsService contract ──
163-
const analytics = registeredServices.get('analytics');
164-
const analyticsResult = await analytics.query({ cube: 'test' });
165-
expect(Array.isArray(analyticsResult.rows)).toBe(true);
166-
expect(Array.isArray(analyticsResult.fields)).toBe(true);
167-
expect(Array.isArray(await analytics.getMeta())).toBe(true);
162+
// ── analytics: deliberately NOT stubbed (#4000) ──
163+
// #3891/#3989 retired the degraded analytics shim so an empty slot is the
164+
// honest signal (route unmounted → 404, discovery `unavailable`). A dev
165+
// stub refilled the slot and the dispatcher, gating on presence alone,
166+
// served its fabricated rows with a 200 — the retired shape, one layer
167+
// down. The slot stays empty; install @objectstack/service-analytics.
168+
expect(registeredServices.has('analytics')).toBe(false);
169+
expect(stubLog[0]).not.toContain('analytics');
168170

169171
// ── Verify IRealtimeService contract ──
170172
const realtime = registeredServices.get('realtime');

packages/plugins/plugin-dev/src/dev-plugin.ts

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -97,16 +97,6 @@ function createAutomationStub() {
9797
}
9898

9999

100-
/** IAnalyticsService — dev stub returning empty results */
101-
function createAnalyticsStub() {
102-
return {
103-
_dev: true, _serviceName: 'analytics',
104-
async query() { return { rows: [], fields: [] }; },
105-
async getMeta() { return []; },
106-
async generateSql() { return { sql: '', params: [] }; },
107-
};
108-
}
109-
110100
/** IRealtimeService — in-memory pub/sub stub */
111101
function createRealtimeStub() {
112102
const subs = new Map<string, Function>();
@@ -266,7 +256,6 @@ const DEV_STUB_FACTORIES: Record<string, () => Record<string, any>> = {
266256
'file-storage': createStorageStub,
267257
'search': createSearchStub,
268258
'automation': createAutomationStub,
269-
'analytics': createAnalyticsStub,
270259
'realtime': createRealtimeStub,
271260
'notification': createNotificationStub,
272261
'ai': createAIStub,
@@ -281,6 +270,24 @@ const DEV_STUB_FACTORIES: Record<string, () => Record<string, any>> = {
281270
'security.fieldMasker': createSecurityFieldMaskerStub,
282271
};
283272

273+
/**
274+
* Core service slots that deliberately get NO dev stub — not even the
275+
* shapeless `{ _dev: true }` fallback the registration loop uses for slots
276+
* without a factory.
277+
*
278+
* `analytics` (#4000): #3891/#3989 retired the degraded analytics shim, making
279+
* an unoccupied slot the honest signal — `/analytics/*` is not mounted, the
280+
* request 404s, and discovery reports `unavailable`. Registering a dev stub
281+
* re-filled that slot and re-created the retired shape one layer down: the
282+
* dispatcher gates on service presence, so a stub was called like a real
283+
* engine and answered 200 with fabricated rows. Dev mode is explicit opt-in
284+
* and the fake was empty, so this was never the security hole the shim was —
285+
* but "the capability is present" must mean the same thing in dev as in
286+
* production. To use analytics locally, install the real engine:
287+
* `@objectstack/service-analytics` runs an InMemory strategy.
288+
*/
289+
const NO_DEV_STUB_SERVICES = new Set<string>(['analytics']);
290+
284291
/**
285292
* Dev Plugin Options
286293
*
@@ -657,11 +664,13 @@ export class DevPlugin implements Plugin {
657664
// dev stub (implementing the interface from packages/spec/src/contracts/)
658665
// so that the full kernel service map is populated and downstream code
659666
// receives correct return types (arrays, booleans, objects — not undefined).
667+
// Exception: NO_DEV_STUB_SERVICES slots stay empty on purpose (#4000).
660668

661669
const stubNames: string[] = [];
662670

663671
for (const svc of CORE_SERVICE_NAMES) {
664672
if (!enabled(svc)) continue;
673+
if (NO_DEV_STUB_SERVICES.has(svc)) continue;
665674
try {
666675
ctx.getService(svc);
667676
// Already registered by a real plugin — skip

packages/runtime/src/dispatcher-plugin.routes.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,31 @@ describe('createDispatcherPlugin — HTTP route registration', () => {
155155
for (const r of ANALYTICS_ROUTES) expect(routes).toContain(r);
156156
});
157157

158+
// [#4000] "Registered" is not the test — `handlerReady` is (ADR-0076 D12).
159+
// plugin-dev used to fill this slot with a stub, which mounted the three
160+
// routes and served its fabricated rows with a 200. The stub is retired,
161+
// and the mount gate now reads the same predicate the domain does, so the
162+
// wire surface can't advertise routes that could only 404.
163+
it('does NOT mount /analytics when the registered analytics service self-declares as a stub', async () => {
164+
const { server, routes } = makeFakeServer();
165+
const plugin = createDispatcherPlugin({ prefix: '/api/v1', securityHeaders: false });
166+
await plugin.start?.(ctxWithServices(server, {
167+
analytics: { _dev: true, query: async () => ({ rows: [], fields: [] }) },
168+
}));
169+
170+
for (const r of ANALYTICS_ROUTES) expect(routes).not.toContain(r);
171+
});
172+
173+
it('DOES mount /analytics for a degraded-but-serving analytics service', async () => {
174+
const { server, routes } = makeFakeServer();
175+
const plugin = createDispatcherPlugin({ prefix: '/api/v1', securityHeaders: false });
176+
await plugin.start?.(ctxWithServices(server, {
177+
analytics: { __serviceInfo: { status: 'degraded' }, query: async () => ({ rows: [] }) },
178+
}));
179+
180+
for (const r of ANALYTICS_ROUTES) expect(routes).toContain(r);
181+
});
182+
158183
it('mounts /analytics unconditionally on a multi-tenant host (kernel-resolver wired)', async () => {
159184
// Host-global mounts, per-project services: presence is a per-request
160185
// question the analytics domain answers (`handled:false` → 404), so the

packages/runtime/src/dispatcher-plugin.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { Plugin, PluginContext, IHttpServer } from '@objectstack/core';
44
import { looksLikeInternalErrorLeak, INTERNAL_ERROR_MESSAGE } from '@objectstack/types';
55
import { DispatcherErrorCode } from '@objectstack/spec/api';
66
import { HttpDispatcher, HttpDispatcherResult } from './http-dispatcher.js';
7+
import { isAnalyticsServiceServeable } from './domains/analytics.js';
78
import { validationFailureDetails, VALIDATION_FAILED_STATUS } from './validation-failure.js';
89
import { buildApiError } from './error-envelope.js';
910
import {
@@ -648,14 +649,23 @@ export function createDispatcherPlugin(config: DispatcherPluginConfig = {}): Plu
648649
// Route via dispatch() (not handleAnalytics directly) so the host
649650
// dispatcher's project-aware kernel swap runs first — the
650651
// per-project kernel owns the `analytics` service.
652+
//
653+
// [#4000] "Registered" is not the test — `handlerReady` is. A
654+
// service that self-declares as a stub (ADR-0076 D12) occupies the
655+
// slot without being the capability, and the domain answers it with
656+
// the same `handled:false` 404 an empty slot gets; mounting routes
657+
// that can only 404 would re-advertise a capability that isn't
658+
// there. Same predicate on both, so the wire surface and the
659+
// handler can't disagree.
651660
const analyticsInstalled = dispatcher.isMultiTenantHost() || await (async () => {
652661
const k: any = kernel;
653662
try {
663+
let svc: unknown;
654664
if (k && typeof k.getServiceAsync === 'function') {
655-
const svc = await k.getServiceAsync('analytics').catch(() => undefined);
656-
if (svc) return true;
665+
svc = await k.getServiceAsync('analytics').catch(() => undefined);
657666
}
658-
return !!k?.getService?.('analytics');
667+
if (!svc) svc = k?.getService?.('analytics');
668+
return isAnalyticsServiceServeable(svc);
659669
} catch {
660670
return false;
661671
}
@@ -690,7 +700,8 @@ export function createDispatcherPlugin(config: DispatcherPluginConfig = {}): Plu
690700
});
691701
} else {
692702
ctx.logger?.info?.(
693-
'[dispatcher] /analytics not mounted — no `analytics` service is registered. ' +
703+
'[dispatcher] /analytics not mounted — no `analytics` service is registered ' +
704+
'(or the registered one self-declares as a stub). ' +
694705
'Install @objectstack/service-analytics to enable the analytics API.',
695706
);
696707
}

0 commit comments

Comments
 (0)