Skip to content

Commit 77fadbf

Browse files
os-zhuangclaude
andauthored
fix(metadata-protocol,objectql)!: retire the degraded analytics shim — empty slot 404s instead of serving unscoped aggregates (#3891) (#3989)
The protocol assembly registered a lightweight `analytics` fallback so POST /api/v1/analytics/query kept answering without service-analytics. Retire it (option A of #3891, per the #3878 archaeology) instead of patching it, because it degraded security semantics, not just features: - Its `query` was unary: the ExecutionContext the dispatcher threads (#2852) was dropped, aggregation reached `engine.aggregate` with no principal, and the security middleware's empty-context branch waved it through — no RLS or tenant predicate was injected. A 200 with rows the caller should never see. - It read a non-contract `filters` key and ignored the canonical `AnalyticsQuery.where`, so a spec-conformant filtered request silently returned a full-table aggregate. - Every analytics security gate had to be built twice (#3770 on the shim vs #3867/#3875 on the real engine), refuting ADR-0076 D10's "duplicates logic only, harmless" assessment. Changes: - `assembleMetadataProtocol` no longer fills the `analytics` slot; the facade's `analyticsQuery`/`getAnalyticsMeta`/`mapAnalyticsOperator` (which existed only to serve the shim) are removed, and the class no longer implements `AnalyticsProtocol`. - `getDiscovery()` stops hardcoding analytics as an always-available kernel service — the pre-#2462 lie the shim was invented to make true. It joins SERVICE_CONFIG: absent ⇒ `enabled:false, unavailable`, no advertised route; registered ⇒ self-declared status honoured. - The dispatcher's `/analytics` domain is untouched: an empty slot takes the existing `handled:false` → 404 ROUTE_NOT_FOUND path (pinned by http-dispatcher.test.ts), which is the honest "install @objectstack/service-analytics" signal. - ADR-0076 D10/D12 annotated with the retirement; services-checklist and data-api docs updated; changeset carries the migration. `os serve` default/full presets and managed environments force the real engine (ALWAYS_ON_CAPABILITIES) and are unaffected. Programmatic embeds without service-analytics now get 404s instead of wrong numbers. Closes #3891. Refs #3878, #2852, #3597, #3770, #3867. Claude-Session: https://claude.ai/code/session_017WZBR9XyhXoqKCU6qQVyjx Co-authored-by: Claude <noreply@anthropic.com>
1 parent 495019b commit 77fadbf

10 files changed

Lines changed: 175 additions & 283 deletions

File tree

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
---
2+
"@objectstack/metadata-protocol": major
3+
"@objectstack/objectql": major
4+
---
5+
6+
fix(metadata-protocol,objectql)!: retire the degraded analytics shim — the `analytics` slot stays empty without service-analytics (#3891, #3878)
7+
8+
The protocol assembly (`assembleMetadataProtocol`, used by both
9+
`MetadataProtocolPlugin` and `ObjectQLPlugin`'s built-in mode) used to register
10+
a lightweight `analytics` fallback so `POST /api/v1/analytics/query` kept
11+
answering on installs without `@objectstack/service-analytics`. That fallback
12+
is **removed**, and with it the facade methods that existed only to serve it:
13+
`ObjectStackProtocolImplementation.analyticsQuery` / `getAnalyticsMeta` (the
14+
class no longer implements `AnalyticsProtocol`).
15+
16+
Why removal instead of repair (#3891):
17+
18+
- **It dropped the caller's ExecutionContext at the door.** The dispatcher
19+
passes `context.executionContext` (#2852), but the shim's `query` was
20+
unary — aggregation reached `engine.aggregate` with no context, the security
21+
middleware's empty-principal branch waved it through, and **no RLS or tenant
22+
predicate was injected**. An authenticated caller got a 200 with rows RLS
23+
would hide.
24+
- **It ignored the contract filter.** `AnalyticsQuery`'s canonical filter field
25+
is `where`; the shim read only a non-contract `filters` key, so a
26+
spec-conformant filtered request silently returned a full-table aggregate.
27+
- **Every security gate had to be built twice** (#3770 on the shim vs
28+
#3867/#3875 on the real engine) — the "duplicates logic only, harmless"
29+
assessment in ADR-0076 D10 did not survive contact with reality.
30+
31+
`getDiscovery()` stops hardcoding analytics as an always-on kernel service —
32+
the entry is now computed from the service registry like every other optional
33+
service (`enabled: false, status: 'unavailable'` and **no advertised route**
34+
when absent), which also removes the pre-#2462 discovery lie the shim was
35+
originally invented to make true.
36+
37+
**Migration.** Deployments that relied on the fallback (programmatic
38+
`createStandaloneStack()` / `createObjectQLKernel()` embeds, hosts whose bundle
39+
doesn't require `analytics`): install `@objectstack/service-analytics` and
40+
mount `AnalyticsServicePlugin` — the real, context-aware engine. Without it,
41+
`/api/v1/analytics/*` now answers **404 ROUTE_NOT_FOUND** (previously: 200 with
42+
unscoped, unfiltered aggregates) and discovery reports
43+
`analytics: { enabled: false, status: 'unavailable' }`. Callers of
44+
`protocol.analyticsQuery(...)` / `protocol.getAnalyticsMeta(...)` must use the
45+
`analytics` service (`kernel.getService('analytics')`) instead. `os serve`
46+
default/full presets and managed environments already force the real engine and
47+
are unaffected.

content/docs/api/data-api.mdx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,10 @@ to set a new name or clear a unique field.
184184

185185
## Analytics
186186

187-
Semantic BI queries using a cube-style API. Available when the analytics service is enabled.
187+
Semantic BI queries using a cube-style API. Provided by `@objectstack/service-analytics`
188+
on deployments without it these endpoints answer `404 ROUTE_NOT_FOUND` and discovery
189+
reports `analytics: { enabled: false, status: "unavailable" }`. (The former kernel-level
190+
degraded fallback was retired — it served unscoped, unfiltered aggregates.)
188191

189192
### `POST /analytics/query`
190193

content/docs/kernel/services-checklist.mdx

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -34,16 +34,16 @@ The ObjectStack protocol defines **16 kernel services** registered via the `Core
3434
┌─────────────────────────────────────────────────────────┐
3535
│ Kernel Layer │
3636
│ ┌──────────────────┐ ┌──────────────────────────────┐ │
37-
│ │ metadata (⚠️) │ │ data + analytics (✅) │ │
37+
│ │ metadata (⚠️) │ │ data (✅) │ │
3838
│ │ In-memory only │ │ ObjectQL example kernel │ │
3939
│ │ DB persistence │ │ Will be rebuilt as plugins │ │
4040
│ │ pending │ │ │ │
4141
│ └──────────────────┘ └──────────────────────────────┘ │
4242
├─────────────────────────────────────────────────────────┤
4343
│ Plugin Layer │
44-
│ All other services: auth, automation, workflow, ui,
45-
│ realtime, notification, ai, i18n, search,
46-
│ file-storage, cache, queue, job
44+
│ All other services: analytics, auth, automation,
45+
workflow, ui, realtime, notification, ai, i18n, │
46+
search, file-storage, cache, queue, job │
4747
│ │
4848
│ Discovery API reports: enabled/unavailable/degraded │
4949
│ per service so clients adapt their UI accordingly │
@@ -58,7 +58,7 @@ The ObjectStack protocol defines **16 kernel services** registered via the `Core
5858
|:--|:-------------|:------------|:--------|:-------|:---------|
5959
| 1 | **metadata** | `core` | 7 | ⚠️ Framework | Kernel (in-memory) |
6060
| 2 | **data** | `required` | 9 | ✅ Implemented | `@objectstack/objectql` |
61-
| 3 | **analytics** | `optional` | 2 | ✅ Implemented | `@objectstack/objectql` |
61+
| 3 | **analytics** | `optional` | 2 | ❌ Plugin Required | `@objectstack/service-analytics` |
6262
| 4 | **auth** | `core` || 🟡 In Development | `@objectstack/plugin-auth` |
6363
| 5 | **ui** | `optional` | 5 | ❌ Plugin Required | TBD plugin |
6464
| 6 | **workflow** | `optional` | 3 | ❌ Plugin Required | TBD plugin |
@@ -173,23 +173,26 @@ The discovery endpoint returns a `services` map so clients know what is availabl
173173

174174
---
175175

176-
## 3. analytics Service ✅ Implemented
176+
## 3. analytics Service — Plugin Required
177177

178178
**Service Name**: `analytics` · **Criticality**: `optional`
179-
**Implementation**: `@objectstack/objectql` (driver-level capability)
180-
**Route Mount**: `/api/v1/analytics`
179+
**Implementation**: `@objectstack/service-analytics` (the only implementation)
180+
**Route Mount**: `/api/v1/analytics` — only served when the plugin registers the service
181181

182-
### Protocol Methods
183-
184-
| Method | Signature | Status |
185-
|:-------|:----------|:------:|
186-
| `analyticsQuery` | `AnalyticsQueryRequest → AnalyticsResultResponse` ||
187-
| `getAnalyticsMeta` | `GetAnalyticsMetaRequest → AnalyticsMetadataResponse` ||
182+
<Callout type="warn">
183+
The kernel-level "degraded analytics fallback" (a lightweight ObjectQL adapter
184+
registered by the protocol assembly) was **retired** (#3891): it dropped the
185+
caller's `ExecutionContext` — aggregates ran without RLS/tenant scoping — and
186+
silently ignored the contract `where` filter. Without
187+
`@objectstack/service-analytics`, `/api/v1/analytics/*` now answers **404** and
188+
discovery reports `analytics: { enabled: false, status: "unavailable" }`.
189+
</Callout>
188190

189-
### Features
191+
### Features (via `@objectstack/service-analytics`)
190192

191-
- Cube semantic queries: measures / dimensions / filters → ObjectQL aggregation
192-
- Auto-generated metadata from SchemaRegistry (numeric → sum/avg, date → time dimensions)
193+
- Cube semantic queries: measures / dimensions / `where` filter → SQL or ObjectQL aggregation
194+
- Context-aware read scoping: per-object RLS/tenant predicates resolved from the request's `ExecutionContext` (fail-closed)
195+
- Cube metadata (`/analytics/meta`), SQL preview (`/analytics/sql`), dataset queries (`/analytics/dataset/query`)
193196

194197
---
195198

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,15 +104,15 @@ The segmentation is **spec/type-level and may start incrementally now** (define
104104

105105
A single `ObjectStackProtocolImplementation` facade — and any `*-protocol` *implementation* package — is the wrong end-state. Verified against source:
106106
- The facade implements only **4 of the 11** contract domains (data, metadata, analytics, feed). The other 7 (realtime / notifications / workflow / ai / i18n / views / permissions) are **not implemented in it** — they belong to their own services or aren't implemented at all. *(Update #1959: the feed domain was retired per ADR-0052 §5 — `IFeedService`, `FeedProtocol`, and the facade's feed forwarding no longer exist, so the facade now implements 3 domains: data, metadata, analytics.)*
107-
- **Analytics uses a deliberate fallback + `replaceService` pattern (NOT a collision/bug — corrected in rev.9)**: `registerService` throws on duplicate (`core/kernel.ts`), so `ObjectQLPlugin` registers a lightweight ~66-line analytics **fallback** (so `/analytics` doesn't 404 in minimal deployments), and `AnalyticsServicePlugin`, when installed, calls **`ctx.replaceService('analytics', …)`** to swap in the full ~1.8k-LOC engine (three strategies incl. native-SQL). With service-analytics present the real engine serves (no shadowing); without it the fallback serves. The fallback duplicates *logic* only (a minor maintenance cost) and is intentional and harmless.
107+
- **Analytics uses a deliberate fallback + `replaceService` pattern (NOT a collision/bug — corrected in rev.9)**: `registerService` throws on duplicate (`core/kernel.ts`), so `ObjectQLPlugin` registers a lightweight ~66-line analytics **fallback** (so `/analytics` doesn't 404 in minimal deployments), and `AnalyticsServicePlugin`, when installed, calls **`ctx.replaceService('analytics', …)`** to swap in the full ~1.8k-LOC engine (three strategies incl. native-SQL). With service-analytics present the real engine serves (no shadowing); without it the fallback serves. The fallback duplicates *logic* only (a minor maintenance cost) and is intentional and harmless. *(Update #3891: "duplicates logic only / harmless" was refuted by evidence — the same security gate had to be built twice (#3770 for the fallback, #3867/#3875 for the real engine), and the fallback dropped the caller's ExecutionContext (aggregates ran without RLS/tenant predicates) and ignored the contract `where` filter. The fallback is **retired**: without service-analytics the `analytics` slot stays empty, `/analytics` answers the dispatcher's existing 404, and discovery reports `unavailable` — the D12 machinery makes the empty slot honest, which removes the fallback's original reason to exist (the discovery hardcode).)*
108108
- ~~**Feed is already delegated** to `IFeedService`; the facade only forwards.~~ *(Retired per ADR-0052 §5 / #1959`IFeedService` and the facade's feed forwarding were removed.)*
109109
- The domain service packages already exist: `service-analytics`, `service-messaging`, `service-realtime`.
110110

111111
Target end-state:
112112
- **"protocol" names ONLY the contract** — the segmented interfaces in `@objectstack/spec/api` (D9). There is **no `*-protocol` implementation package**.
113113
- **`DataProtocol`** impl → engine-adjacent / transport (thin wire-normalizers).
114114
- **`MetadataProtocol`** impl → stays in **`@objectstack/metadata-protocol`** (name **retained** — already published; renaming churns downstream for ~0 benefit). The package's *content* converges to the metadata-management impl (it owns `sys_metadata`). The `protocol` in the name is a deliberate, low-cost naming exception from being published — the real contract lives in `@objectstack/spec/api`, not here. (Open Question #7, resolved.)
115-
- **Analytics / Realtime / Notification / …** → each domain's *full* impl lives in its existing service package. For analytics specifically, the `ObjectQLPlugin` fallback **must be preserved or consciously dropped** (it prevents `/analytics` 404 for deployments without service-analytics) — **not blindly deleted**. The engine keeps only the minimal fallback it deliberately provides. *(Feed was dropped entirely per ADR-0052 §5 / #1959 — no service package, no facade forwarding.)*
115+
- **Analytics / Realtime / Notification / …** → each domain's *full* impl lives in its existing service package. For analytics specifically, the `ObjectQLPlugin` fallback **must be preserved or consciously dropped** (it prevents `/analytics` 404 for deployments without service-analytics) — **not blindly deleted**. The engine keeps only the minimal fallback it deliberately provides. *(Feed was dropped entirely per ADR-0052 §5 / #1959 — no service package, no facade forwarding.)* *(Update #3891: consciously dropped — see the fallback bullet above. The facade's `analyticsQuery`/`getAnalyticsMeta` went with it, so the facade now implements 2 domains (data, metadata) and analytics has exactly one implementation, in service-analytics — this bullet's end-state for the domain.)*
116116
- **The transport/dispatcher routes each contract-slice to the owning service** (it already resolves services by name) — no central facade class.
117117

118118
This **refines D1** (the `metadata-protocol` package was an intermediate, not the end-state) and **completes D9**. Executed at the cross-repo window with D7 / Step 2.
@@ -143,7 +143,7 @@ Decision: each capability plugin registers its routes as a **normalized handler*
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

146-
**Supersedes the rev.9 analytics conclusion**: the fix for the analytics fallback is to **mark it honestly (this D12)**, not "preserve-or-delete".
146+
**Supersedes the rev.9 analytics conclusion**: the fix for the analytics fallback is to **mark it honestly (this D12)**, not "preserve-or-delete". *(Update #3891: superseded in turn for the analytics fallback specifically — honest labelling was necessary but not sufficient. The fallback's `degraded` label was accurate about capability, yet nothing in it disclosed that aggregates ran WITHOUT the caller's RLS/tenant scoping and that the contract `where` filter was ignored; an authorized caller still got a 200 with wrong (over-broad) numbers. A fallback may degrade features, never security semantics — so it was retired, and the "no `/analytics` 404 regression" goal above is deliberately abandoned for this slot: the 404 IS the honest signal. D12's marker/discovery machinery stays, and is what makes the now-empty slot report `unavailable`.)*
147147

148148
**Execution**: framework (marker convention + `svcAvailable` respects it + discovery schema `stub` status) and console (read the honest status) land **together at the cross-repo window** — the console reads `discovery.services`, so this is a cross-repo contract change.
149149

packages/metadata-protocol/src/plugin.ts

Lines changed: 20 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@
55
*
66
* Owns what `ObjectQLPlugin` historically assembled inline: the
77
* `ObjectStackProtocolImplementation` construction + `protocol` service
8-
* registration, the metadata-storage platform objects, and the lightweight
9-
* `analytics` fallback. Registering it NEXT TO `ObjectQLPlugin` (with
8+
* registration and the metadata-storage platform objects. (The lightweight
9+
* `analytics` fallback that used to ride here was retired in #3891 — see
10+
* {@link assembleMetadataProtocol}.) Registering it NEXT TO `ObjectQLPlugin` (with
1011
* `registerProtocol: false` on the engine plugin) makes `@objectstack/objectql`
1112
* effectively protocol-free at boot-assembly level — the engine plugin keeps
1213
* only protocol CONSUMERS (DB hydration + authored hook/action rebind, both of
@@ -22,7 +23,6 @@
2223
*/
2324

2425
import type { Plugin, PluginContext } from '@objectstack/core';
25-
import { SERVICE_SELF_INFO_KEY, type ServiceSelfInfo } from '@objectstack/spec/api';
2626
import {
2727
SysMetadataObject,
2828
SysMetadataHistoryObject,
@@ -73,12 +73,19 @@ export function createMetadataProtocolPlugin(options: MetadataProtocolPluginOpti
7373

7474
/**
7575
* The ONE protocol assembly (ADR-0076 Step 2 PR-C): metadata-storage platform
76-
* objects + `ObjectStackProtocolImplementation` as the `protocol` service +
77-
* the D12 `degraded` analytics fallback. Called by
78-
* {@link createMetadataProtocolPlugin} (delegated mode) AND by
76+
* objects + `ObjectStackProtocolImplementation` as the `protocol` service.
77+
* Called by {@link createMetadataProtocolPlugin} (delegated mode) AND by
7978
* `ObjectQLPlugin`'s built-in convenience mode (`registerProtocol !== false`)
8079
* — single source, two mounts, identical result.
8180
*
81+
* The `analytics` slot is deliberately NOT filled here (#3891 / #3878,
82+
* superseding the ADR-0076 D10/D12 "preserve the fallback" stance): the
83+
* degraded shim dropped the caller's ExecutionContext (no RLS/tenant
84+
* predicates) and ignored the contract `where` filter, returning full-table
85+
* aggregates with a 200. An empty slot degrades honestly instead — the
86+
* dispatcher's `/analytics` domain answers 404 and discovery reports
87+
* `unavailable` until `AnalyticsServicePlugin` registers the real engine.
88+
*
8289
* @returns the protocol shim, so the engine-side caller can arm its
8390
* mutation-rebind subscription synchronously.
8491
*/
@@ -119,42 +126,13 @@ export function assembleMetadataProtocol(
119126
ctx.registerService('protocol', protocolShim);
120127
ctx.logger.info('Protocol service registered (MetadataProtocolPlugin)');
121128

122-
// Lightweight `analytics` fallback mapped onto the protocol shim's
123-
// `analyticsQuery` — kept with the protocol assembly so `/analytics`
124-
// keeps answering on installs without service-analytics. Honest
125-
// capabilities (ADR-0076 D12): self-identifies as `degraded`;
126-
// AnalyticsServicePlugin replaces it (ctx.replaceService) with the
127-
// real engine.
128-
ctx.registerService('analytics', {
129-
[SERVICE_SELF_INFO_KEY]: {
130-
status: 'degraded',
131-
handlerReady: true,
132-
message: 'Lightweight ObjectQL analytics fallback — install @objectstack/service-analytics for the full engine',
133-
} satisfies ServiceSelfInfo,
134-
// HttpDispatcher passes the raw POST body (AnalyticsQuery
135-
// shape); the shim's `analyticsQuery` expects the wrapped
136-
// `{ cube, query }` envelope and returns its own `{ success,
137-
// data }` — reshape in, unwrap out (one level), exactly as the
138-
// historical inline adapter did.
139-
query: async (body: any) => {
140-
const envelope = body && typeof body === 'object' && 'query' in body && 'cube' in body
141-
? body
142-
: { cube: body?.cube, query: body };
143-
const result = await protocolShim.analyticsQuery(envelope);
144-
if (result && typeof result === 'object' && 'success' in result && 'data' in result) {
145-
return (result as any).data;
146-
}
147-
return result;
148-
},
149-
getMeta: async () => ({
150-
cubes: [],
151-
message: 'Analytics meta endpoint not implemented by ObjectQL adapter',
152-
}),
153-
generateSql: async (_body: any) => ({
154-
sql: null,
155-
message: 'Analytics SQL generation not implemented by ObjectQL adapter',
156-
}),
157-
});
129+
// NO `analytics` fallback rides here anymore (#3891 / #3878). The
130+
// degraded shim this assembly used to register dropped the request's
131+
// ExecutionContext (aggregates ran without RLS/tenant predicates)
132+
// and ignored the contract filter field `where` — a 200 with wrong
133+
// numbers. The slot now stays empty: `/analytics/*` answers 404
134+
// (ROUTE_NOT_FOUND) and discovery reports the service unavailable
135+
// until @objectstack/service-analytics registers the real engine.
158136

159137
return protocolShim;
160138
}

0 commit comments

Comments
 (0)