From 60099e506e4aa3e228f5ebc846a60c4573e4ba52 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 3 Aug 2026 12:35:18 +0000 Subject: [PATCH] fix(rest): strip read decorations before validating a dataset query MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every widget on every dataset-bound dashboard failed with "Dataset query failed: 400 Bad Request — Invalid dataset definition." POST /analytics/dataset/query resolves a saved `datasetName` through `getMetaItems`, and the metadata READ path stamps its spec-validation verdict `_diagnostics` onto every document it serves. Since #4001 closed the metadata schemas, `DatasetSchema.parse()` rejects unrecognized keys rather than dropping them — so the route handed a served document back to the schema that produced it and got `unrecognized_keys: ["_diagnostics"]`. The 400 blamed the author for a key the server had just added. This is exactly what `stripReadDecorations` exists to prevent, and the failure `spec/kernel/metadata-read-decorations.ts` already documents from the cold-boot flow bind (cloud#971). Strip before validating. Applied on BOTH branches, not only the `datasetName` read: the Studio dataset preview posts its draft inline, and that draft is the document the designer GET-loaded, decorations included. A hand-authored draft never carries these keys, so the strip is a no-op there. The ADR-0010 provenance envelope (`_packageId`, `_provenance`, …) is not a read decoration and still survives. Regression coverage for the saved-dataset path was the gap that let this ship — every existing case passed the dataset inline, so nothing exercised the read. Tests now cover resolve-by-name, the inline decorated draft, the 404, and a genuinely malformed saved dataset (still 400). Also bumps the vendored console pin to objectui@f5bc4c78be76 via scripts/bump-objectui.sh, with the changeset it generates. Verified in a real browser against the showcase app: before, all 8 widgets of showcase_ops_dashboard rendered the 400; after, all 8 render live data, with zero failed responses and zero console errors. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01QuQBvLsJWuYFAYdkqDbM4N --- .changeset/console-f5bc4c78be76.md | 21 +++++++ .../dataset-query-strip-read-decorations.md | 36 +++++++++++ .objectui-sha | 2 +- packages/rest/src/analytics-routes.test.ts | 62 ++++++++++++++++++- packages/rest/src/rest-server.ts | 17 +++++ 5 files changed, 135 insertions(+), 3 deletions(-) create mode 100644 .changeset/console-f5bc4c78be76.md create mode 100644 .changeset/dataset-query-strip-read-decorations.md diff --git a/.changeset/console-f5bc4c78be76.md b/.changeset/console-f5bc4c78be76.md new file mode 100644 index 0000000000..6c0a92fc46 --- /dev/null +++ b/.changeset/console-f5bc4c78be76.md @@ -0,0 +1,21 @@ +--- +"@objectstack/console": minor +--- + +Console (objectui) refreshed to `f5bc4c78be76`. Frontend changes in this range: + +Derived from the changesets objectui declared over the range — 11 releasing of 11 changesets added across 31 non-merge commits; omitted: 20 commits carrying no changeset (they ship no package code). + +- **minor** — Field widgets are finally told when their field fails validation, and the props slot that carries it takes the name the published contract gives it (objectui#3222). (objectui `56409c28c`) +- **minor** — Retire `validation` from the action-param contract — it was declared on both halves, read by neither, and rejected outright by the server (objectui#3201). (objectui `f833d3ae4`) +- **patch** — Five metadata designers stop rendering keys `@objectstack/spec` rejects, and start rendering the keys it declares (objectui#3275, objectui#3281). (objectui `8ff3ad7b8`) +- **patch** — The Page block inspector's conditional-visibility control now authors `visibleWhen`, and says "Visible when" while doing it (objectui#3229). (objectui `8e02ad7f2`) +- **patch** — The record discussion panel no longer shows the PREVIOUS record's comments and activity (objectui#3268). (objectui `a8aa57663`) +- **patch** — The form renderer's built-in `select` branch stops saying "No options available" in English to non-English sessions (objectui#3263). (objectui `a7651e640`) +- **patch** — The record discussion panel now says "loading" while it is loading, instead of "No comments yet" (objectui#3209). (objectui `12bf6691e`) +- **patch** — The legacy `page-header` alias stops advertising `description` as an authorable key (objectui#3226). (objectui `d2363e710`) +- **patch** — The option widgets' "this list cannot be filled" message now has one source, and it is translated (objectui#3231). (objectui `825bbe33c`) +- **patch** — `ToolPreview` stops advertising retired `ToolSchema` flags (objectui#3236). (objectui `30ac2e1ee`) +- **patch** — `TextAreaField`'s mobile fullscreen flag converges on its one real producer (objectui#3232). (objectui `a321fa461`) + +objectui range: `785b8a5d432c...f5bc4c78be76` diff --git a/.changeset/dataset-query-strip-read-decorations.md b/.changeset/dataset-query-strip-read-decorations.md new file mode 100644 index 0000000000..ff9071945b --- /dev/null +++ b/.changeset/dataset-query-strip-read-decorations.md @@ -0,0 +1,36 @@ +--- +"@objectstack/rest": patch +--- + +fix(rest): dataset queries stop rejecting their own read-time annotation + +Every widget on every dataset-bound dashboard failed with + +``` +Dataset query failed: 400 Bad Request — Invalid dataset definition. +``` + +The dataset itself was fine. `POST /analytics/dataset/query` resolves a saved +`datasetName` through `getMetaItems`, and the metadata READ path stamps the +spec-validation verdict `_diagnostics` onto every document it serves. Since +#4001 closed the metadata schemas, `DatasetSchema.parse()` rejects unrecognized +keys instead of dropping them — so the route handed a served document back to +the very schema that produced it and got `unrecognized_keys: ["_diagnostics"]` +for its trouble. The 400 blamed the author for a key the server had just added. + +This is the failure mode `stripReadDecorations` exists to prevent, and the one +`spec/kernel/metadata-read-decorations.ts` already documents from the cold-boot +flow bind (cloud#971): *a served body is not a valid input to the schema that +produced it.* The route now strips read decorations before validating. + +Stripped on **both** branches, not only the `datasetName` read: the Studio +dataset preview posts its draft inline, and that draft is the document the +designer GET-loaded — decorations and all. A hand-authored draft never carries +these keys, so the strip is a no-op there. The ADR-0010 provenance envelope +(`_packageId`, `_provenance`, `_lock`, …) is deliberately *not* a read +decoration and still survives the round-trip. + +Regression coverage for the saved-dataset path was the gap that let this ship — +every existing case passed the dataset inline, so nothing exercised the read. +The route's tests now cover resolve-by-name, the inline decorated draft, the +404, and a genuinely malformed saved dataset (still a 400). diff --git a/.objectui-sha b/.objectui-sha index f84237d630..682a305812 100644 --- a/.objectui-sha +++ b/.objectui-sha @@ -1 +1 @@ -785b8a5d432cf009389a1a9180fdac2a8297543f +f5bc4c78be7629ea0c585b5be42bc9f23682532c diff --git a/packages/rest/src/analytics-routes.test.ts b/packages/rest/src/analytics-routes.test.ts index 1128c23b67..9645a4bda7 100644 --- a/packages/rest/src/analytics-routes.test.ts +++ b/packages/rest/src/analytics-routes.test.ts @@ -30,10 +30,10 @@ const inlineDataset = { const selection = { dimensions: ['region'], measures: ['revenue'] }; /** Build a RestServer with an optional analytics provider (positional arg #15). */ -function buildServer(analyticsProvider?: any) { +function buildServer(analyticsProvider?: any, protocol: any = mockProtocol()) { const server = mockServer(); const rest = new RestServer( - server as any, mockProtocol() as any, { api: { requireAuth: false } } as any, + server as any, protocol as any, { api: { requireAuth: false } } as any, undefined, undefined, undefined, undefined, undefined, undefined, undefined, undefined, undefined, undefined, undefined, analyticsProvider, @@ -90,6 +90,64 @@ describe('POST /analytics/dataset/query', () => { expect(res.body.code).toBe('VALIDATION_FAILED'); }); + // ── saved datasets (`datasetName`) ───────────────────────────────────────── + // Every case above passes the dataset INLINE, which is why the read path + // below shipped broken: `getMetaItems` stamps the read-time `_diagnostics` + // verdict onto every served item, and `DatasetSchema` is closed (#4001), so + // the strict re-parse rejected our own decoration and answered 400 "Invalid + // dataset definition." for every saved dataset — i.e. every dashboard widget. + it('resolves a saved dataset by name and strips read decorations before parsing', async () => { + const served = { + ...inlineDataset, + _packageId: 'com.example.showcase', + _diagnostics: { valid: true }, + }; + const protocol = { ...mockProtocol(), getMetaItems: vi.fn().mockResolvedValue({ items: [served] }) }; + const queryDataset = vi.fn().mockResolvedValue({ rows: [{ region: 'NA', revenue: 100 }], fields: [] }); + const { route } = buildServer(async () => ({ queryDataset }), protocol); + const res = mockRes(); + await route!.handler({ method: 'POST', params: {}, headers: {}, body: { datasetName: 'sales', selection } } as any, res); + + expect(res.statusCode).toBe(200); + expect(res.body.rows).toEqual([{ region: 'NA', revenue: 100 }]); + const passed = queryDataset.mock.calls[0][0]; + expect(passed).not.toHaveProperty('_diagnostics'); + // The ADR-0010 provenance envelope is NOT a read decoration — it survives. + expect(passed._packageId).toBe('com.example.showcase'); + }); + + // The Studio dataset preview posts the draft INLINE — and that draft is the + // document the designer GET-loaded, decorations included. + it('strips read decorations from an INLINE dataset too', async () => { + const queryDataset = vi.fn().mockResolvedValue({ rows: [], fields: [] }); + const { route } = buildServer(async () => ({ queryDataset })); + const res = mockRes(); + const served = { ...inlineDataset, _diagnostics: { valid: true }, _provenance: 'package' }; + await route!.handler({ method: 'POST', params: {}, headers: {}, body: { dataset: served, selection } } as any, res); + + expect(res.statusCode).toBe(200); + expect(queryDataset.mock.calls[0][0]).not.toHaveProperty('_diagnostics'); + }); + + it('returns 404 for an unknown datasetName', async () => { + const protocol = { ...mockProtocol(), getMetaItems: vi.fn().mockResolvedValue({ items: [] }) }; + const { route } = buildServer(async () => ({ queryDataset: vi.fn() }), protocol); + const res = mockRes(); + await route!.handler({ method: 'POST', params: {}, headers: {}, body: { datasetName: 'nope', selection } } as any, res); + expect(res.statusCode).toBe(404); + expect(res.body.code).toBe('NOT_FOUND'); + }); + + it('still rejects a saved dataset that is genuinely malformed', async () => { + const served = { ...inlineDataset, measures: [{ name: 'x', aggregate: 'not_a_real_agg' }], _diagnostics: { valid: false } }; + const protocol = { ...mockProtocol(), getMetaItems: vi.fn().mockResolvedValue({ items: [served] }) }; + const { route } = buildServer(async () => ({ queryDataset: vi.fn() }), protocol); + const res = mockRes(); + await route!.handler({ method: 'POST', params: {}, headers: {}, body: { datasetName: 'sales', selection } } as any, res); + expect(res.statusCode).toBe(400); + expect(res.body.code).toBe('VALIDATION_FAILED'); + }); + it('maps a dataset D-C compile error to 400 (undeclared relationship)', async () => { const queryDataset = vi.fn().mockRejectedValue(new Error("dimension \"region\" references relationship \"account\" via \"account.region\", but \"account\" is not declared in the dataset's `include`.")); const { route } = buildServer(async () => ({ queryDataset })); diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 5fc41e6c9e..f55dbaac58 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -12,6 +12,7 @@ import { DataProtocol, MetadataProtocol } from '@objectstack/spec/api'; import type { FieldErrorCode } from '@objectstack/spec/api'; import { PUBLIC_FORM_SERVER_MANAGED_FIELDS } from '@objectstack/spec/security'; import { PLURAL_TO_SINGULAR } from '@objectstack/spec/shared'; +import { stripReadDecorations } from '@objectstack/spec/kernel'; import type { DroppedFieldsEvent } from '@objectstack/spec/data'; import { preferredLocaleFromHeader } from '@objectstack/spec/system'; import type { ISecurityService } from '@objectstack/spec/contracts'; @@ -5993,6 +5994,22 @@ export class RestServer { return res.status(400).json({ code: 'VALIDATION_FAILED', message: 'Provide body.dataset (inline) or body.datasetName.' }); } + // A SERVED document is not a valid input to the schema that + // produced it: the read path stamps `_diagnostics` on every + // item `getMetaItems` returns, and since #4001 + // `DatasetSchema` is CLOSED — so the parse below rejected + // our OWN annotation with `unrecognized_keys`, answering + // 400 "Invalid dataset definition." for every saved dataset, + // i.e. every widget on every dataset-bound dashboard. Same + // shape as the cold-boot flow bind (cloud#971). + // + // Stripped on BOTH branches, not just the `datasetName` + // read: the Studio dataset preview posts its draft INLINE, + // and that draft is the document the designer GET-loaded — + // decorations and all. A genuinely hand-authored draft + // never carries these keys, so the strip is a no-op there. + dataset = stripReadDecorations(dataset); + // Validate against the spec schema so a malformed draft // yields a clean 400 instead of a runtime throw. try {