From 0bc8bfee22e26c7cd98667f5e1155e68af03a5f7 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 11:33:18 +0000 Subject: [PATCH 1/2] wip: cross-reference the two $orderby declarations + dual-declaration pin Claude-Session: https://claude.ai/code/session_019srGWGCBBCBHqcDoRZpQRh Co-authored-by: Claude --- .../odata-orderby-dual-declaration.test.ts | 149 ++++++++++++++++++ packages/spec/src/api/odata.zod.ts | 50 ++++++ packages/spec/src/data/data-engine.zod.ts | 19 +++ 3 files changed, 218 insertions(+) create mode 100644 packages/spec/src/api/odata-orderby-dual-declaration.test.ts diff --git a/packages/spec/src/api/odata-orderby-dual-declaration.test.ts b/packages/spec/src/api/odata-orderby-dual-declaration.test.ts new file mode 100644 index 00000000000..b9ca5b27579 --- /dev/null +++ b/packages/spec/src/api/odata-orderby-dual-declaration.test.ts @@ -0,0 +1,149 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#18977] `$orderby` is declared TWICE, and the two declarations are + * COMPLEMENTARY REFUSALS — each accepts exactly what the other rejects. + * + * | declaration | accepts | refuses | + * |:---|:---|:---| + * | `ODataQuerySchema.$orderby` (`api/odata.zod.ts`) | `string`, `string[]` | the record maps, `SortNode[]` | + * | `QueryTransportParamsSchema.$orderby` = `DataEngineSortSchema` (`data/data-engine.zod.ts`) | the record maps, `SortNode[]` | `string`, `string[]` — deliberately, #18704 | + * + * Neither file pointed at the other, so reading one of them carefully and + * completely still produced the wrong answer about the other — measured, at a + * price: objectui#9554 was filed, triaged, graded and dispatched against a + * shipped `object-grid` producer that had been sending the canonical shape all + * along, because the filing seat read the OData declaration and correctly + * quoted it. The cross-references landed in both files with this card; this + * file is their MECHANICAL half. + * + * ## What this pins, and what it deliberately does not + * + * It pins the two accept sets AS THEY ARE, and their disjointness. It is not + * an argument that either set is right: + * + * - §1/§2 hold each side's accept set, so widening or narrowing either one + * turns this red and lands the author on the cross-reference that explains + * why the gap is a decision rather than a defect. Widening the TRANSPORT + * side is the one the source argues against in its own words — lowering a + * sort expression means PARSING, and a second parser beside the door's is + * how one rule gets two implementations that disagree. + * - §3 is the disjointness itself: no value parses under both. That is the + * property a reader cannot get from either file alone, and the one that + * makes "I read the declaration" insufficient. + * - §4 says which of the two grades a query bag, through the slot the REST + * door actually parses (`FindDataRequestSchema.query`). + * + * ⛔ It pins nothing about what the RUNTIME serves. The OData string forms are + * not unserved — `normalizeSortNodes` (`@objectstack/metadata-protocol`) reads + * them at the GET querystring ingress and for in-process `findData`. What this + * file measures is the two SCHEMAS, which is where the card's trap lives. + */ + +import { describe, it, expect } from 'vitest'; +import { ODataQuerySchema } from './odata.zod'; +import { FindDataRequestSchema } from './protocol.zod'; +import { DataEngineSortSchema, QueryTransportParamsSchema } from '../data/data-engine.zod'; + +/** The two shapes `ODataQuerySchema` declares and the transport schema refuses. */ +const ODATA_SPELLINGS: ReadonlyArray = [ + ["the 'field direction' expression", 'name desc'], + ["the '-field' shorthand", '-created_at'], + ['the expression array', ['name desc', 'email asc']], + ['a single-element expression array', ['name']], +]; + +/** The three shapes `DataEngineSortSchema` declares and the OData schema refuses. */ +const TRANSPORT_SPELLINGS: ReadonlyArray = [ + ['the asc/desc record map', { name: 'desc' }], + ['the 1/-1 record map', { name: 1 }], + ['the SortNode array', [{ field: 'name', order: 'desc' }]], +]; + +describe('[#18977] $orderby is declared twice — the two accept sets', () => { + describe('§1 ODataQuerySchema.$orderby — the OData URL-convention vocabulary', () => { + it.each(ODATA_SPELLINGS)('accepts %s', (_label, value) => { + const parsed = ODataQuerySchema.safeParse({ $orderby: value }); + expect(parsed.success).toBe(true); + }); + + it.each(TRANSPORT_SPELLINGS)('refuses %s, at the $orderby member', (_label, value) => { + const parsed = ODataQuerySchema.safeParse({ $orderby: value }); + expect(parsed.success).toBe(false); + if (parsed.success) return; + expect(parsed.error.issues.map((i) => i.path.join('.'))).toContain('$orderby'); + }); + }); + + describe('§2 DataEngineSortSchema — what QueryTransportParamsSchema.$orderby is', () => { + it.each(TRANSPORT_SPELLINGS)('accepts %s', (_label, value) => { + expect(DataEngineSortSchema.safeParse(value).success).toBe(true); + expect(QueryTransportParamsSchema.safeParse({ $orderby: value }).success).toBe(true); + }); + + it.each(ODATA_SPELLINGS)('refuses %s, at the $orderby member', (_label, value) => { + expect(DataEngineSortSchema.safeParse(value).success).toBe(false); + const parsed = QueryTransportParamsSchema.safeParse({ $orderby: value }); + expect(parsed.success).toBe(false); + if (parsed.success) return; + expect(parsed.error.issues.map((i) => i.path.join('.'))).toContain('$orderby'); + }); + + it('refuses the OData spelling on `sort` too — the bare transport alias of the same slot', () => { + // A reader who works around the `$orderby` refusal by re-spelling the key + // gets the same answer: the refusal is on the VALUE, not on the spelling. + for (const [, value] of ODATA_SPELLINGS) { + expect(QueryTransportParamsSchema.safeParse({ sort: value }).success).toBe(false); + } + }); + }); + + describe('§3 the two accept sets are DISJOINT — this is the trap', () => { + it('no declared $orderby value parses under both', () => { + const every = [...ODATA_SPELLINGS, ...TRANSPORT_SPELLINGS]; + const bothAccept = every.filter(([, value]) => + ODataQuerySchema.safeParse({ $orderby: value }).success + && DataEngineSortSchema.safeParse(value).success); + expect(bothAccept.map(([label]) => label)).toEqual([]); + }); + + it('every declared $orderby value parses under exactly one of them — neither set is empty', () => { + // The lit control for the emptiness above: a disjointness assertion is + // also satisfied by two schemas that accept nothing at all. + const every = [...ODATA_SPELLINGS, ...TRANSPORT_SPELLINGS]; + const accepted = every.map(([, value]) => + Number(ODataQuerySchema.safeParse({ $orderby: value }).success) + + Number(DataEngineSortSchema.safeParse(value).success)); + expect(accepted).toEqual(every.map(() => 1)); + }); + }); + + describe('§4 which one grades a query bag', () => { + // `POST /data/:object/query` parses its body through this schema and answers + // `400 VALIDATION_FAILED` on a refusal (`rest-server.ts`), so this is the + // declaration an author's stored query bag is actually judged against. + const findInput = (query: Record) => ({ object: 't', query: { ...query, object: 't' } }); + + it.each(TRANSPORT_SPELLINGS)('FindDataRequest.query accepts %s on $orderby', (_label, value) => { + expect(FindDataRequestSchema.safeParse(findInput({ $orderby: value })).success).toBe(true); + }); + + it.each(ODATA_SPELLINGS)('FindDataRequest.query refuses %s on $orderby', (_label, value) => { + const parsed = FindDataRequestSchema.safeParse(findInput({ $orderby: value })); + expect(parsed.success).toBe(false); + if (parsed.success) return; + expect(parsed.error.issues.map((i) => i.path.join('.'))).toContain('query.$orderby'); + }); + + it('ODataQuerySchema grades nothing here — it is not on any path into FindDataRequest', () => { + // The canonical AST key is the one the output carries, whichever declared + // spelling arrived: the transport spelling folds onto `orderBy`. + const parsed = FindDataRequestSchema.safeParse(findInput({ $orderby: { created_at: 'desc' } })); + expect(parsed.success).toBe(true); + if (!parsed.success) return; + expect((parsed.data as { query: { orderBy?: unknown } }).query.orderBy) + .toEqual([{ field: 'created_at', order: 'desc' }]); + expect((parsed.data as { query: Record }).query.$orderby).toBeUndefined(); + }); + }); +}); diff --git a/packages/spec/src/api/odata.zod.ts b/packages/spec/src/api/odata.zod.ts index 30399fd71ab..b25169449f6 100644 --- a/packages/spec/src/api/odata.zod.ts +++ b/packages/spec/src/api/odata.zod.ts @@ -71,6 +71,47 @@ import { z } from 'zod'; * System query options defined by OData v4 specification. * These are URL query parameters that control the query execution. * + * ## [#18977] This schema grades no runtime door — and a SECOND declaration of + * the same parameter names does + * + * The declaration that grades a query bag is `QueryTransportParamsSchema` + * (`../data/data-engine.zod.ts`), reached from `FindDataRequestSchema.query` + * through `QueryWithTransportSchema`. This one is the OData v4 URL-convention + * VOCABULARY and nothing parses through it: measured on this tree, its only + * consumers are the `OData.buildUrl` helper at the foot of this file and this + * file's own unit test — no route, no ingress, no normalizer. + * + * ⚠️ On `$orderby` the two declarations are COMPLEMENTARY REFUSALS: each + * accepts exactly what the other rejects, so reading either one carefully and + * completely still produces the wrong answer about the other. Measured with + * `safeParse` on both, one tree: + * + * | `$orderby` value | here | `QueryTransportParamsSchema.$orderby` | + * |:---|:---|:---| + * | `'name desc'` / `'-created_at'` | accepted | REFUSED | + * | `['name desc', 'email asc']` | accepted | REFUSED | + * | `[{field, order}]` | REFUSED | accepted | + * | `{name: 'asc'}` / `{name: 1}` | REFUSED | accepted | + * + * Both halves are pinned in `odata-orderby-dual-declaration.test.ts`, which is + * the mechanical half of this cross-reference: widening or narrowing either + * side turns it red and lands the author here. + * + * ⛔ The gap is NOT closed by widening one side to match the other. #18704 + * settled which spelling is canonical and why the transport schema refuses + * these two: lowering a sort EXPRESSION means PARSING, and a second parser + * beside the door's is how one rule gets two implementations that disagree + * (the paragraph above `QueryTransportParamsSchema` states it verbatim). + * Changing either accept set is a decision, not a tidy-up. + * + * ⭐ What the string forms are not is unserved. `normalizeSortNodes` + * (`@objectstack/metadata-protocol`) is the one shared ingress normalizer + * behind `GET /data/:object`, the export route and in-process `findData`, and + * it reads `'name desc'`, `'-created_at'` and the `string[]` form — so a + * querystring spelled the OData way works, while the same bag sent as a + * `POST /data/:object/query` body answers `400 VALIDATION_FAILED`. The + * difference is the DOOR, and neither door is this schema. + * * @see https://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part2-url-conventions.html#sec_SystemQueryOptions */ import { lazySchema } from '../shared/lazy-schema'; @@ -115,6 +156,15 @@ export const ODataQuerySchema = lazySchema(() => z.object({ * @example "name" * @example "revenue desc" * @example "country asc, revenue desc" + * + * ⛔ [#18977] These two shapes are exactly the ones + * `QueryTransportParamsSchema.$orderby` (`DataEngineSortSchema`, + * `../data/data-engine.zod.ts`) DELIBERATELY refuses, and that is the + * declaration a query bag is graded against. Sent as a + * `POST /data/:object/query` body this spelling answers + * `400 VALIDATION_FAILED`; sent on the querystring it is parsed by + * `normalizeSortNodes` at the ingress and works. See the cross-reference on + * the schema above before writing either shape into stored metadata. */ $orderby: z.union([ z.string(), // "name desc" diff --git a/packages/spec/src/data/data-engine.zod.ts b/packages/spec/src/data/data-engine.zod.ts index 1d95c4acd37..683e600b5a4 100644 --- a/packages/spec/src/data/data-engine.zod.ts +++ b/packages/spec/src/data/data-engine.zod.ts @@ -830,6 +830,25 @@ const TransportCountValueSchema = lazySchema(() => z.union([z.boolean(), z.enum( * its `string[]` form. They fail the parse at the member that carries them * rather than reaching the AST as a string the engine would have to re-read. * + * ⚠️ [#18977] `$orderby` is declared a SECOND time, and the other declaration + * accepts exactly the two shapes this one refuses: `ODataQuerySchema.$orderby` + * (`../api/odata.zod.ts`) is `string | string[]` and refuses this one's record + * maps and `SortNode[]`. It cannot contradict this schema at a DOOR — measured + * on this tree, nothing parses through it: its only consumers are its own + * `OData.buildUrl` helper and its own unit test. It contradicts it in a READER, + * which is the cost already paid: objectui#9554 was filed, triaged and + * dispatched against a shipped producer that had been sending the canonical + * shape all along, because a competent seat read the OTHER declaration, quoted + * it correctly, and had no signal that this one exists. The two accept sets are + * disjoint and pinned as such in + * `../api/odata-orderby-dual-declaration.test.ts`. + * + * ⛔ Closing that gap by widening either side is a decision, not a tidy-up — + * and widening THIS one is precisely the second parser the paragraph above + * refuses. What serves the string forms is `normalizeSortNodes` at the + * `@objectstack/metadata-protocol` ingress (the GET querystring path, the + * export route and in-process `findData`), not a schema. + * * ⛔ Declaring the narrower structured form ALONE would have turned live * traffic into a `400` — measured: the body-form AST array on * `POST /data/:object/query`, pinned by `#7390 §3` in From c7e22addb4cf9435bc3cb401e2a3545f3fce986d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 11:39:06 +0000 Subject: [PATCH 2/2] docs(spec): cross-reference the two `$orderby` declarations and pin their disjoint accept sets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ODataQuerySchema.$orderby` (`string | string[]`) and `QueryTransportParamsSchema.$orderby` (`DataEngineSortSchema`: the record maps and `SortNode[]`) are complementary refusals — each accepts exactly what the other rejects — and neither pointed at the other, so reading one of them carefully and completely still produced the wrong answer about the other. No accept set moves. Both docblocks now name the other declaration, say which one grades a query bag (`FindDataRequestSchema.query` via `QueryWithTransportSchema`), and say what actually parses the string forms (`normalizeSortNodes` at the metadata-protocol ingress, not a schema). `src/api/odata-orderby-dual-declaration.test.ts` holds both accept sets and their disjointness, so widening or narrowing either side turns red. Claude-Session: https://claude.ai/code/session_019srGWGCBBCBHqcDoRZpQRh Co-authored-by: Claude --- ...rderby-dual-declaration-cross-reference.md | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 .changeset/18977-orderby-dual-declaration-cross-reference.md diff --git a/.changeset/18977-orderby-dual-declaration-cross-reference.md b/.changeset/18977-orderby-dual-declaration-cross-reference.md new file mode 100644 index 00000000000..128263f0386 --- /dev/null +++ b/.changeset/18977-orderby-dual-declaration-cross-reference.md @@ -0,0 +1,23 @@ +--- +"@objectstack/spec": patch +--- + +`$orderby` is declared twice — `ODataQuerySchema.$orderby` and `QueryTransportParamsSchema.$orderby` now cross-reference each other, and a pin holds the two accept sets apart (#18977). + +Clause-②: no. No accept set moves and no export is added, removed or renamed: the change is two docblocks in published source (`src/api/odata.zod.ts`, `src/data/data-engine.zod.ts`) plus a new pin test. Measured — `check:generated` reports all 16 generated artifacts up to date, `check:api-surface` and `check:authorable-surface` included. + +The two declarations are **complementary refusals**: each accepts exactly what the other rejects, and neither pointed at the other, so reading one of them carefully and completely still produced the wrong answer about the other. + +| `$orderby` value | `ODataQuerySchema` | `QueryTransportParamsSchema` (`DataEngineSortSchema`) | +|:---|:---|:---| +| `'name desc'` / `'-created_at'` | accepted | REFUSED | +| `['name desc', 'email asc']` | accepted | REFUSED | +| `[{field, order}]` | REFUSED | accepted | +| `{name: 'asc'}` / `{name: 1}` | REFUSED | accepted | + +- **Which one grades a query bag**: `QueryTransportParamsSchema`, reached from `FindDataRequestSchema.query` through `QueryWithTransportSchema` — the schema `POST /data/:object/query` parses its body against. `ODataQuerySchema` grades no runtime door: measured on this tree, its only consumers are the `OData.buildUrl` helper in its own file and its own unit test. +- **The refusal on the transport side is deliberate and stays** — `#18704` settled it: lowering an OData sort *expression* means PARSING, and a second parser beside the door's is how one rule gets two implementations that disagree. Widening either side to close the gap is a decision, not a tidy-up, so this change closes the **reader's** half only. +- **The string forms are not unserved.** `normalizeSortNodes` (`@objectstack/metadata-protocol`) reads `'name desc'`, `'-created_at'` and the `string[]` form at the shared ingress behind `GET /data/:object`, the export route and in-process `findData`. A querystring spelled the OData way works; the same bag sent as a `POST /data/:object/query` body answers `400 VALIDATION_FAILED`. The difference is the door, and neither door is `ODataQuerySchema`. +- **The cost this repairs was already paid.** objectui#9554 was filed, triaged, graded and dispatched against a shipped `object-grid` producer that had been sending the canonical shape all along, because the filing seat read the OData declaration and quoted it correctly. + +`src/api/odata-orderby-dual-declaration.test.ts` is the mechanical half: 25 cases pinning each side's accept set, their disjointness (with the lit control that neither set is empty), and which of the two `FindDataRequestSchema.query` is graded by. Widening or narrowing either declaration turns it red and lands the author on the cross-reference.