fix(analytics,rest): 五族 dataset 拒收自带 DATASET_INVALID/400 信封,路由的 message 正则名单收到只剩一条

.changeset/analytics-dataset-refusal-envelope.md

@@ -0,0 +1,73 @@
+---
+"@objectstack/service-analytics": patch
+"@objectstack/rest": patch
+"@objectstack/spec": patch
+---
+
+fix(analytics,rest): five dataset refusals declare `DATASET_INVALID` / 400 themselves, and the route's message-sniffing list shrinks to one entry (#5367)
+
+`POST /analytics/dataset/query` answered `400 DATASET_INVALID` for six error
+families because the route recognised their **prose**, not because the errors
+said anything about themselves. #5352 gave the catch an ADR-0112 envelope branch
+(`error.code` + a 4xx `error.status`, read first) and had to leave a hardcoded
+list of message substrings behind it, since all six producers were still bare
+`throw new Error(…)`:
+
+```
+/not declared in the dataset|not backed by a declared relationship|
+ not supported by the v1 dataset runtime|read-scope-sql|
+ not a selected dimension or measure|is not a subset of the selected dimensions/
+```
+
+That made the HTTP status of six families a property of their wording.
+Rephrasing `dataset-compiler`'s "is not declared in the dataset's `include`" —
+no logic change — moved that refusal from 400 to 500, i.e. re-opened #5352 for a
+different family, and no test and no gate would have gone red. Prime Directive
+#12 permits an accommodation like that only while it is declared, loud, tested
+**and removable on a schedule**; #5366 delivered the first three and nothing
+carried the fourth.
+
+**Five producers now declare their own verdict.** A new
+`dataset-refusal.ts` in `@objectstack/service-analytics` exports
+`datasetInvalidError` — the same shape as that package's existing
+`invalidFilterError` (`INVALID_FILTER` / 400) and `assertDimensionFields`
+(`INVALID_FIELD` / 400) — and five sites throw through it:
+
+- `dataset-compiler.ts` — a measure whose aggregate the v1 runtime cannot lower;
+  a dimension/measure traversing a relationship path the dataset never declared
+  in `include`;
+- `dataset-executor.ts` — an `order` key that is not a selected dimension or
+  measure; a `totals` grouping that is not a subset of the selected dimensions;
+- `native-sql-strategy.ts` — a join outside the dataset's declared allowlist.
+
+Their five entries are gone from the route's list, which is now a single
+`read-scope-sql` test.
+
+**`read-scope-sql` deliberately stays.** Its ten fail-closed refusals are RLS
+read-scope lowering failures whose inputs are an admin-authored policy and a
+compiler-generated join alias — not caller input — so `DATASET_INVALID` ("your
+request is invalid") may well be the wrong verdict and choosing the right one is
+a separate judgement, still tracked by #5367. Deleting the entry before that
+judgement lands would regress those ten from `400 DATASET_INVALID` to 500.
+
+**No outward behaviour change for the five.** They answered
+`400 DATASET_INVALID` before and answer `400 DATASET_INVALID` now, with the same
+message; what changed is the mechanism, from message-matching to the producer's
+own declaration. The one visible difference is for a bare `Error` that merely
+*resembles* one of those messages: it is no longer promoted to a 400. That is the
+point — a phrase is no longer a classification.
+
+`DATASET_INVALID` is registered in `ERROR_CODE_LEDGER` under
+`@objectstack/service-analytics` as well as `@objectstack/rest` (provenance, per
+ADR-0112 D3; the code itself is unchanged and the union does not grow), and the
+constructor types it as `RegisteredErrorCode` so an unregistered code is a
+compile error rather than a body some route rejects at runtime.
+
+Coverage: `dataset-refusal-envelope.test.ts` (service-analytics) pins each of the
+five refusals against its real producer — the refusal SET first, green before and
+after, then the envelope; `analytics-dataset-refusal-envelope.test.ts` (rest)
+drives all five end-to-end through a real `AnalyticsService` with positive
+controls on both the aggregate and raw-SQL paths; and
+`analytics-filter-refusal-envelope.test.ts` pins the deletion in both directions
+— the five messages answer 400 when enveloped and 500 when bare, so re-adding a
+regex entry turns it red.

packages/rest/src/analytics-dataset-dimension-gate.test.ts

@@ -295,16 +295,38 @@ describe('[#5520] the 500 body no longer ships driver internals', () => {
     expect(a.body.code).toBe('INVALID_FILTER');
     expect(String(a.body.message)).toMatch(/\$sortOf/);
 
-    // …② and the transitional message list still answers 400 DATASET_INVALID
-    // with its message intact. #5367 owns that list; this change is 5xx-only.
+    // …② and so does a dataset refusal, with its message intact.
+    //
+    // [#5367] This half used to send a BARE `Error` reading "… is not declared
+    // in the dataset." and rely on the route's transitional message list to
+    // classify it. #5367 enveloped that producer (`dataset-compiler` now throws
+    // `datasetInvalidError`) and deleted the list entry, so the same refusal is
+    // now carried by branch ① — same outward answer, chosen by reading the error
+    // instead of by matching its prose. This change is still 5xx-only.
     const b = await post(
       buildRoute(async () =>
-        throwingAnalytics(new Error('[dataset-compiler] dimension "region" is not declared in the dataset.')),
+        throwingAnalytics(
+          Object.assign(
+            new Error('[dataset-compiler] dimension "region" references relationship path "account" via "account.region", but "account" is not declared in the dataset\'s `include`.'),
+            { code: 'DATASET_INVALID', status: 400 },
+          ),
+        ),
       ),
       { dataset, selection: { measures: ['account_count'], dimensions: ['industry'] } },
     );
     expect(b.statusCode).toBe(400);
     expect(b.body.code).toBe('DATASET_INVALID');
     expect(String(b.body.message)).toMatch(/not declared in the dataset/);
+
+    // …③ and the ONE message-list entry #5367 deliberately left in place still
+    // answers 400 for `read-scope-sql`'s bare fail-closed refusals.
+    const c = await post(
+      buildRoute(async () =>
+        throwingAnalytics(new Error('[read-scope-sql] unsupported operator "$regex" on "owner" (fail-closed).')),
+      ),
+      { dataset, selection: { measures: ['account_count'], dimensions: ['industry'] } },
+    );
+    expect(c.statusCode).toBe(400);
+    expect(c.body.code).toBe('DATASET_INVALID');
   });
 });

packages/rest/src/analytics-dataset-refusal-envelope.test.ts

@@ -0,0 +1,256 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * [#5367] `POST /analytics/dataset/query` — five dataset refusals reach the
+ * caller as `400 DATASET_INVALID` because their PRODUCER says so, not because
+ * this route recognised their prose.
+ *
+ * ## The seam, and why this file boots the REAL analytics service
+ *
+ * #5352 / PR #5366 gave the route an envelope branch (`error.code` + a 4xx
+ * `error.status`, read first) and left a transitional list of message substrings
+ * behind it, because six refusal families were still bare `throw new Error(…)`:
+ *
+ * ```
+ * /not declared in the dataset|not backed by a declared relationship|
+ *  not supported by the v1 dataset runtime|read-scope-sql|
+ *  not a selected dimension or measure|is not a subset of the selected dimensions/
+ * ```
+ *
+ * With that list in place the HTTP status of six families was a property of their
+ * **wording**: rephrasing `dataset-compiler`'s "is not declared in the dataset's
+ * `include`" — no logic change — dropped the refusal from 400 to 500, and no test
+ * and no gate would have gone red. Prime Directive #12 allows such an
+ * accommodation only while it is declared, loud, tested **and removable on a
+ * schedule**; #5366 delivered the first three. #5367 is the schedule: five
+ * producers now throw `datasetInvalidError` (`DATASET_INVALID` / 400) and their
+ * five entries are gone from the list.
+ *
+ * The claim has two halves and either alone reads as fixed:
+ *
+ *   - **B** — the producer throws the envelope. Pinned per site, against the real
+ *     producer, in `service-analytics`'s `dataset-refusal-envelope.test.ts`.
+ *   - **A** — the route classifies on that envelope rather than on the message.
+ *     Pinned in `analytics-filter-refusal-envelope.test.ts`, including the
+ *     five deleted entries asserted in both directions.
+ *
+ * A unit test on either side can be green while an author still sees a 500. So
+ * this file asserts the SEAM: the analytics provider is a real `AnalyticsService`
+ * compiling a real dataset, the error crossing into the catch is the one the real
+ * `dataset-compiler` / `dataset-executor` / `native-sql-strategy` throws, and
+ * nothing here asserts a shape it also constructs.
+ *
+ * ## Reverse verification, direction predicted BEFORE running
+ *
+ * Restore ANY of the five `throw new Error(…)` calls in `service-analytics` (and
+ * rebuild it — this file exercises the BUILT package) and that family's case here
+ * goes RED with `500 ANALYTICS_QUERY_FAILED`, because the route no longer carries
+ * a message entry that would rescue it. The direction is plain red, not the
+ * inverted/extra-diagnostic shapes: the canonical envelope branch is FIRST in the
+ * catch and the fallback that used to answer for these families is gone, so
+ * "producer stops declaring" has exactly one outward consequence. Confirmed by
+ * running it (see the PR).
+ *
+ * Positive controls sit next to the refusals so a case cannot pass merely because
+ * the wiring never reached the producer.
+ */
+
+import { describe, it, expect, vi } from 'vitest';
+import type { Logger } from '@objectstack/spec/contracts';
+import { AnalyticsService } from '@objectstack/service-analytics';
+import { RestServer } from './rest-server';
+
+// ── harness (the shape `analytics-filter-refusal-envelope.test.ts` uses) ──────
+
+function mockServer() {
+  return {
+    get: vi.fn(), post: vi.fn(), put: vi.fn(), delete: vi.fn(), patch: vi.fn(),
+    use: vi.fn(), listen: vi.fn().mockResolvedValue(undefined), close: vi.fn().mockResolvedValue(undefined),
+  };
+}
+function mockProtocol() {
+  return {
+    getDiscovery: vi.fn().mockResolvedValue({ version: 'v0', endpoints: {} }),
+    getMetaTypes: vi.fn().mockResolvedValue([]),
+    getMetaItems: vi.fn().mockResolvedValue([]),
+  };
+}
+function mockRes() {
+  const res: any = { statusCode: 200, body: undefined };
+  res.status = vi.fn((c: number) => { res.statusCode = c; return res; });
+  res.json = vi.fn((b: any) => { res.body = b; return res; });
+  res.end = vi.fn(() => res);
+  return res;
+}
+
+/** Build a RestServer over an analytics provider (positional arg #15). */
+function buildRoute(analyticsProvider?: any) {
+  const rest = new RestServer(
+    mockServer() as any, mockProtocol() as any, { api: { requireAuth: false } } as any,
+    undefined, undefined, undefined, undefined, undefined, undefined, undefined,
+    undefined, undefined, undefined, undefined,
+    analyticsProvider,
+  );
+  (rest as any).resolveExecCtx = async () => ({ userId: 'test-user' });
+  rest.registerRoutes();
+  return rest.getRoutes().find((r) => r.method === 'POST' && r.path.endsWith('/analytics/dataset/query'))!;
+}
+
+async function post(route: any, body: unknown) {
+  const res = mockRes();
+  await route.handler({ method: 'POST', params: {}, headers: {}, body } as any, res);
+  return res;
+}
+
+const silent: Logger = { debug() {}, info() {}, warn() {}, error() {} };
+
+/**
+ * A real `AnalyticsService` on the ObjectQL aggregate path — one fixed bucket, so
+ * a selection that gets far enough to touch data answers 200. That is what makes
+ * the refusals meaningful: they fail on the dataset/selection, on a route that
+ * demonstrably works otherwise.
+ */
+function aggregateAnalytics(): AnalyticsService {
+  return new AnalyticsService({
+    logger: silent,
+    queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }),
+    executeAggregate: async () => [{ stage: 'won', revenue: 100 }],
+    isRegisteredObject: () => true,
+  });
+}
+
+/** A real `AnalyticsService` on the raw-SQL path — the only one that enforces the join allowlist. */
+function nativeAnalytics(): AnalyticsService {
+  return new AnalyticsService({
+    logger: silent,
+    queryCapabilities: () => ({ nativeSql: true, objectqlAggregate: false, inMemory: false }),
+    executeRawSql: async () => [{ stage: 'won', revenue: 100 }],
+    isRegisteredObject: () => true,
+  });
+}
+
+/** A valid single-object dataset — no `include`, so nothing here needs a join. */
+const dataset = {
+  name: 'pipeline',
+  label: 'Pipeline',
+  object: 'crm_opportunity',
+  dimensions: [{ name: 'stage', field: 'stage', type: 'string' }],
+  measures: [{ name: 'revenue', aggregate: 'sum', field: 'amount' }],
+};
+const selection = { dimensions: ['stage'], measures: ['revenue'] };
+
+// ─────────────────────────────────────────────────────────────────────────────
+
+describe('[#5367] a dataset refusal answers 400 DATASET_INVALID from its own envelope', () => {
+  /**
+   * One row per entry #5367 removed from the route's message list, driven through
+   * the real producer. `listEntry` records the substring that used to classify it
+   * — the audit trail that the deletion and this coverage are the same set.
+   */
+  const CASES: Array<{
+    name: string;
+    listEntry: string;
+    body: unknown;
+    analytics: () => AnalyticsService;
+    message: RegExp;
+  }> = [
+    {
+      name: 'dataset-compiler: an aggregate the v1 runtime cannot lower',
+      listEntry: 'not supported by the v1 dataset runtime',
+      analytics: aggregateAnalytics,
+      body: {
+        dataset: {
+          ...dataset,
+          measures: [{ name: 'names', aggregate: 'string_agg', field: 'name' }],
+        },
+        selection: { dimensions: ['stage'], measures: ['names'] },
+      },
+      message: /measure "names" uses aggregate "string_agg" which is not supported by the v1 dataset runtime/,
+    },
+    {
+      name: 'dataset-compiler: a dimension traversing an undeclared relationship path',
+      listEntry: 'not declared in the dataset',
+      analytics: aggregateAnalytics,
+      body: {
+        dataset: {
+          ...dataset,
+          include: [],
+          dimensions: [{ name: 'region', field: 'account.region', type: 'string' }],
+        },
+        selection: { dimensions: ['region'], measures: ['revenue'] },
+      },
+      message: /"account" is not declared in the dataset's `include`/,
+    },
+    {
+      name: 'dataset-executor: an order key that is not selected',
+      listEntry: 'not a selected dimension or measure',
+      analytics: aggregateAnalytics,
+      body: { dataset, selection: { ...selection, order: { profit: 'desc' } } },
+      message: /order key\(s\) "profit" — not a selected dimension or measure/,
+    },
+    {
+      name: 'dataset-executor: a totals grouping outside the selection',
+      listEntry: 'is not a subset of the selected dimensions',
+      analytics: aggregateAnalytics,
+      body: { dataset, selection: { ...selection, totals: { groupings: [['region']] } } },
+      message: /totals grouping \[region\] is not a subset of the selected dimensions/,
+    },
+    {
+      // The caller-shaped trigger: the DATASET declares no `include`, and the
+      // SELECTION names a dotted dimension, which `lookupMember`'s synthetic
+      // relation fallback turns into a join at alias `account`.
+      name: 'native-sql-strategy: a selection naming a join outside the allowlist',
+      listEntry: 'not backed by a declared relationship',
+      analytics: nativeAnalytics,
+      body: { dataset, selection: { dimensions: ['account.region'], measures: ['revenue'] } },
+      message: /join "account" is not backed by a declared relationship on cube "pipeline"/,
+    },
+  ];
+
+  for (const c of CASES) {
+    it(`${c.name} → 400 DATASET_INVALID`, async () => {
+      const route = buildRoute(async () => c.analytics());
+      const res = await post(route, c.body);
+
+      expect(res.statusCode).toBe(400);
+      expect(res.body.code).toBe('DATASET_INVALID');
+      // The message survives intact so the author can act on it, and the body is
+      // the 4xx shape (`message`), not the 5xx one (`error`).
+      expect(String(res.body.message)).toMatch(c.message);
+      expect(res.body.error).toBeUndefined();
+      // The defect, asserted as the defect rather than as the fix.
+      expect(res.statusCode).not.toBe(500);
+      expect(res.body.code).not.toBe('ANALYTICS_QUERY_FAILED');
+    });
+  }
+
+  it('covers exactly the five entries #5367 deleted from the message list', () => {
+    expect(CASES.map((c) => c.listEntry).sort()).toEqual([
+      'is not a subset of the selected dimensions',
+      'not a selected dimension or measure',
+      'not backed by a declared relationship',
+      'not declared in the dataset',
+      'not supported by the v1 dataset runtime',
+    ]);
+  });
+
+  it('POSITIVE control (aggregate path): the same wiring, a valid selection → 200 with rows', async () => {
+    const route = buildRoute(async () => aggregateAnalytics());
+    const res = await post(route, { dataset, selection });
+    expect(res.statusCode).toBe(200);
+    expect(res.body.rows).toEqual([{ stage: 'won', revenue: 100 }]);
+  });
+
+  it('POSITIVE control (raw-SQL path): a DECLARED relationship still joins → 200 with rows', async () => {
+    // The allowlist case's twin: `include: ['account']` makes the same dotted
+    // selection legal, so case ⑤ above is a verdict about the allowlist rather
+    // than about dotted members being rejected outright.
+    const route = buildRoute(async () => nativeAnalytics());
+    const res = await post(route, {
+      dataset: { ...dataset, include: ['account'] },
+      selection: { dimensions: ['account.region'], measures: ['revenue'] },
+    });
+    expect(res.statusCode).toBe(200);
+    expect(res.body.rows).toEqual([{ stage: 'won', revenue: 100 }]);
+  });
+});