Skip to content

Commit 6908830

Browse files
os-zhuangclaude
andauthored
fix(objectql)!: engine.find/findOne refuse an unmaterializable formula ORDER BY (#7095) (#7337)
* fix(objectql)!: `engine.find`/`findOne` refuse an unmaterializable formula ORDER BY (#7095) #6994 closed the SORT axis at the REST ingress (`assertSortFieldsExist`, `400 INVALID_SORT`), covering everything reaching `findData`. A caller reaching `engine.find()` / `engine.findOne()` directly passed through none of it, and a `formula` ORDER BY there was dropped in silence. Measured on this change's base, real `ObjectQL` over a driver that really sorts: engine.find(o, { orderBy: [{ field: <formula>, order: 'asc' }] }) -> C A E B D engine.find(o, { orderBy: [{ field: <formula>, order: 'desc' }] }) -> C A E B D asc === desc (byte-identical) Ruled 2026-08-10 on #7095: refuse at the public boundary with guidance prose, never a silent drop. `assertOrderByIsMaterializable` refuses on both entry points with the same `400 INVALID_SORT` and the same remedy sentence the two ingress verdicts emit — pinned as an equality across all three doors, since separate wordings is how #4256 and #6673 drifted apart. The tolerance was to survive only behind a pinned internal path, and only if a MEASURED internal call site relied on it. The sweep found none: every hardcoded internal sort names a real stored column, and no shipped object declares a `formula` field. So no internal path shipped, and a negative pin keeps one off the public options shape. The one author-reachable consumer is why ingress-only was not tenable: a saved report's `query.orderBy` is forwarded verbatim into `engine.find` by `plugin-reports`. One path deliberately does NOT become a refusal — a nested `expand` sort raises it inside `expandRelatedRecords`, whose pre-existing graceful-degradation catch swallows every expand failure, so that path moves from silent to observable (a warning naming the field and the fix) rather than refusing. Reversing that backstop is #3821's decision, not this card's; it is measured and pinned as-is. The ingress gate is untouched, and the engine door judges only the third verdict — unknown and dotted names still reach the driver from a direct call, because refusing those is a posture change on two further axes. Registered in the ADR-0087 step-17 ledger as `engine-find-formula-order-by-refused`; artifacts regenerated. Refs #7095, #6994, #6924, #4226, #4256, #3821, ADR-0087, ADR-0112 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HJgGahRqaYPRJ2oKmk9Czc * test(objectql): type the #7095 query-options sites instead of erasing them to `any` The four call sites the #7095 pins added tripped the #4918 query-options-erasure ratchet (test surface 249 -> 253). Fixed at the call sites, per the rule's own prescription — the ceiling is unchanged and no pin is weakened. Three were ON-contract and are now typed: - the `it.each` sort table is `Array<[string, NonNullable<EngineQueryOptions['orderBy']>]>`, so the three refused sorts are checked as the well-formed `SortNode[]` they are. It is the FIELD they name that the engine refuses, never their shape, and an `as any` there would have erased the one channel that enforces `{ field, order }` on a direct engine call — the `direction`-vs-`order` mistake #4674 is about. - both `expand` sites drop the assertion entirely: `EngineQueryOptions.expand` is `Record<string, QuerySchema>`, so the nested `{ orderBy }` was always assignable and the cast was never buying anything. One is DELIBERATELY off-contract — the negative pin that smuggles an opt-out flag onto the public options bag — and is now `as unknown as EngineQueryOptions` rather than a bare `as any`: it names the contract being bypassed, keeps the rest of the call type-checked, and greps as an intentional act. That is exactly the case #4918 carved the spelling out for, since the assertion's whole subject is that the engine rejects the unknown key. Refs #7095, #4918, #4674, #4721 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HJgGahRqaYPRJ2oKmk9Czc --------- Co-authored-by: claude[bot] <noreply@anthropic.com>
1 parent 69f1a5f commit 6908830

7 files changed

Lines changed: 492 additions & 32 deletions

File tree

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
---
2+
"@objectstack/objectql": major
3+
"@objectstack/metadata-protocol": patch
4+
"@objectstack/spec": patch
5+
---
6+
7+
<!-- adr-0087: registered engine-find-formula-order-by-refused -->
8+
9+
fix(objectql)!: `engine.find` / `engine.findOne` refuse an ORDER BY they cannot materialise (#7095)
10+
11+
`engine.find()` and `engine.findOne()` are a **public API**, and an `orderBy`
12+
naming a `formula` field — which used to return rows successfully, in an
13+
arbitrary order — now **throws `400 INVALID_SORT`**.
14+
15+
#6994 closed this at the REST ingress (`assertSortFieldsExist`), covering
16+
everything that reaches `findData`: the list route, `POST /data/:object/query`,
17+
the export route and the RPC dispatcher. A caller reaching the engine directly
18+
passed through none of it. Measured on the base of this change, real `ObjectQL`
19+
over a driver that really sorts:
20+
21+
```
22+
engine.find(o, { orderBy: [{ field: <formula>, order: 'asc' }] }) -> C A E B D
23+
engine.find(o, { orderBy: [{ field: <formula>, order: 'desc' }] }) -> C A E B D
24+
asc === desc (byte-identical)
25+
```
26+
27+
A `formula` value is computed on read, so no driver materialises a column for
28+
it: the ORDER BY reached the driver, found nothing, and the unknown-column
29+
backstop returned the rows unordered under a success — carrying the very values
30+
they were asked to be ordered by. With `limit`, "the latest N" was an arbitrary
31+
N that no amount of inspecting the response could reveal.
32+
33+
- FROM `orderBy: [{ field: '<formula field>' }]` → TO: denormalise the value
34+
onto the object (a stored field, written when the source changes) and sort by
35+
that. This is the same remedy, in the same words, that the REST door has
36+
prescribed since #6924 / #6994 and that the SEARCH axis prescribes since
37+
#6673 — a caller refused at two doors is not sent two different ways.
38+
39+
**`summary` / rollup fields are NOT affected** and still sort in both
40+
directions: they get a real, maintained column. The family this refuses is
41+
`formula`, not "computed" — widening it to the spec's `COMPUTED_VALUE_TYPES`
42+
(the *write* contract) would break two types that work, and a control test pins
43+
that.
44+
45+
**Who was actually reaching this.** The #7095 ruling required the internal-caller
46+
tolerance to survive only behind a pinned internal path, and only if a *measured*
47+
internal call site relied on it. The sweep of every in-tree `orderBy` reaching
48+
the engine directly — hooks, flows, reports, queue/job adapters, sharing,
49+
metadata loaders, expand sub-reads — found **none**: every hardcoded internal
50+
sort names a real stored column (`created_at`, `updated_at`, `version`,
51+
`priority`, `scheduled_for`, `started_at`, `next_run_at`, `recorded_at`, `id`),
52+
and no shipped object in the repo declares a `formula` field at all. So **no
53+
internal path shipped**, and there is no flag to opt back into the drop — a
54+
negative test pins that the public options shape refuses one.
55+
56+
The one **author-reachable** consumer is why leaving this at ingress was not
57+
tenable: a saved report's `query.orderBy` is forwarded verbatim into
58+
`engine.find` by `plugin-reports`, bypassing the ingress gate entirely. A report
59+
authored to sort by a formula field used to run and return an arbitrary order;
60+
it now fails loudly with the remedy in the message.
61+
62+
**One path deliberately does NOT become a refusal.** A nested `expand` sort
63+
raises this same error inside `expandRelatedRecords`, but that sub-read sits in a
64+
pre-existing graceful-degradation `catch` which swallows *every* expand failure
65+
and retains the raw foreign keys. That path therefore moves from **silent** to
66+
**observable** — a warning naming the field and the fix — rather than refusing.
67+
Reversing that backstop is a decision about all expand failure modes (#3821) and
68+
is not ridden in on this change; it is measured and pinned as-is.
69+
70+
**What did NOT change:** the ingress gate is untouched — same message, same
71+
`unknown` > `dotted` > unmaterializable precedence, same `param` name that the
72+
engine cannot know. The engine door judges only the third verdict: unknown and
73+
dotted sort names still reach the driver from a direct call exactly as before,
74+
because refusing those is a posture change on two further axes rather than a
75+
free extension of this one. Reading a formula field, and the projection axis'
76+
`SELECT *` tolerance, are also untouched.

docs/protocol-upgrade-guide.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,15 @@ No mechanical rewrite exists, in either direction. The refused values carry no r
446446

447447
This is a RUNTIME registration API, not stored metadata, so — like `hook-context-session-roles-retired` at this step — there is no `sys_metadata` row for the D2 chain to rewrite and the ledger entry is the notification channel. One metadata surface reaches it INDIRECTLY and is the reason this is not purely a code-side note: a `record-change` flow's start node forwards `config.objectName` verbatim into `registerHook` (`RecordChangeTrigger.start`), so a flow authored with a blank `objectName` used to bind a trigger to EVERY object in the tenant. It now fails to bind instead, loudly — the automation engine's per-flow bind guard warns and the `kernel:bootstrapped` binding audit re-reports it — which is the correct end state, but it is an observable change for that flow. #6573, #4281, #4001, #5928, ADR-0078.
448448
- Done when: No `registerHook` call site passes an empty `object` target, and none passes an `excludeObjects` list covering every name in its `object` list. Every `record-change` flow start node declares a non-blank `config.objectName`, or omits the key if the flow is genuinely meant to fire on every object. Boot completes with no "[ObjectQL] Hook ... declares an empty `object` target" throw and no "[record-change] ... not bound" warning naming a flow you expect to fire.
449+
- **`engine-find-formula-order-by-refused`**`engine.find(object, { orderBy }) and engine.findOne(object, { orderBy }) naming a `formula` field — the direct engine path, not the REST ingress` → denormalise the value onto the object (a stored field, written when the source changes) and sort by that — the same remedy the REST ingress has prescribed since #6924 / #6994; a `summary` field is unaffected and still sorts, because it gets a real maintained column
450+
- Why not automatic: #4226 / #4256 / #6994 closed the SORT axis at the REST ingress (`assertSortFieldsExist`, `400 INVALID_SORT`), which covers everything reaching `findData`: the list route, `POST /data/:object/query`, the export route and the RPC dispatcher. A caller reaching `engine.find()` / `engine.findOne()` DIRECTLY passed through none of it, and a `formula` ORDER BY there was dropped in silence. Measured on a real driver: `asc` and `desc` came back BYTE-IDENTICAL, in insertion order, under a success, with the rows carrying the very values they were asked to be ordered by. No column exists to order by (a formula is computed on read, so no driver materialises one), so the ORDER BY reached the driver, found nothing, and the unknown-column backstop returned the rows unordered.
451+
452+
Ruled 2026-08-10 on #7095: an ORDER BY the engine cannot apply is a 4xx with guidance prose at the public boundary, never a silent drop — the same direction as the analytics dataset refusal envelope and the #6924 sort-hint prescription. The engine's documented internal-caller tolerance (`assertProjectionFieldsExist`'s docblock) was to survive only behind a pinned internal path, and only if a MEASURED internal call site relied on it. The #7095 sweep of every in-tree `orderBy` reaching the engine directly — hooks, flows, reports, queue/job adapters, sharing, metadata loaders, expand sub-reads — found NONE: every hardcoded internal sort names a real stored column (`created_at`, `updated_at`, `version`, `priority`, `scheduled_for`, `started_at`, `next_run_at`, `recorded_at`, `id`), and no shipped object in the repo declares a `formula` field at all. So no internal path shipped, and there is no flag to opt back into the drop.
453+
454+
This is a CODE-path API, not stored metadata, so — like `hook-register-empty-object-target-refused` at this step — there is no `sys_metadata` row for the D2 chain to rewrite and the ledger entry is the notification channel. No mechanical rewrite exists in either direction: the platform cannot invent the stored column the remedy prescribes, and it must not sort post-hoc instead — `driver.find` has already applied `limit` / `offset`, so re-sorting after the formulas are evaluated would reorder an ARBITRARY PAGE, which looks correct on small result sets and is wrong the moment pagination is involved.
455+
456+
ONE AUTHOR-REACHABLE SURFACE reaches this indirectly and is why it is not purely a code-side note: a saved report's `query.orderBy` (`sys_saved_report`) is forwarded verbatim into `engine.find` by `plugin-reports`, bypassing the ingress gate. A report authored to sort by a formula field used to run and return rows in an arbitrary order; it now fails loudly, with the remedy in the message. One further path is deliberately NOT a refusal: a nested `expand` sort raises this refusal inside `expandRelatedRecords`, whose pre-existing graceful-degradation `catch` swallows every expand failure and retains the raw foreign keys — so that path moves from silent to OBSERVABLE (a warning naming the field and the fix) rather than refusing. Reversing that backstop is a separate decision on all expand failure modes. #7095, #6994, #6924, #4226, #4256, #3821, ADR-0112.
457+
- Done when: No `engine.find` / `engine.findOne` call site sorts by a `formula` field, and no saved report's `query.orderBy` names one — grep your report definitions for an `orderBy` field whose object declares it as a `formula`, and denormalise it onto a stored column written when the source changes. A `summary` / rollup field needs no action: it has a real maintained column and sorts correctly. Reads complete with no `INVALID_SORT` naming a formula field, and no "Failed to expand relationship field" warning whose error text names one.
449458

450459
---
451460

packages/metadata-protocol/src/protocol.ts

Lines changed: 49 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5112,16 +5112,37 @@ export class ObjectStackProtocolImplementation implements
51125112
* member of the family with no door — which is why a `formula` field
51135113
* reached a driver that has no column for it.
51145114
*
5115-
* SCOPE, stated because it is a real limit and not an oversight: this is an
5116-
* INGRESS gate, so it covers what reaches {@link findData} — the REST list
5117-
* route, `POST /data/:object/query`, the export route (which funnels its
5118-
* `$orderby` through here) and the RPC dispatcher. An internal caller that
5119-
* reaches `engine.find()` directly — hooks, flows, reports, expand
5120-
* sub-reads — still gets the silent drop, exactly as the projection and
5121-
* search axes note for themselves. Closing that half means deciding whether
5122-
* `engine.find` REFUSES or keeps its deliberate internal-caller tolerance,
5123-
* which is an engine-core contract decision rather than a gate fix; it is
5124-
* tracked separately.
5115+
* SCOPE: this is an INGRESS gate, so it covers what reaches {@link findData}
5116+
* — the REST list route, `POST /data/:object/query`, the export route (which
5117+
* funnels its `$orderby` through here) and the RPC dispatcher.
5118+
*
5119+
* [#7095] It is no longer the ONLY door for this verdict, and the half it
5120+
* cannot reach is now closed rather than merely noted. A caller reaching
5121+
* `engine.find()` / `engine.findOne()` directly — hooks, flows, reports,
5122+
* expand sub-reads — used to get the silent drop;
5123+
* `assertOrderByIsMaterializable` (`@objectstack/objectql`, `engine.ts`)
5124+
* refuses it there with the SAME `400 INVALID_SORT` and the same remedy
5125+
* sentence this gate emits, ruled on #7095 (an ORDER BY the engine cannot
5126+
* apply is a refusal with guidance prose, never a silent drop). What made
5127+
* leaving it at ingress untenable is that the direct path is AUTHOR-
5128+
* reachable, not merely internal: a saved report's `query.orderBy` is
5129+
* forwarded verbatim into `engine.find` (`plugin-reports`), and it never
5130+
* passes through here.
5131+
*
5132+
* ONE EDGE, measured and deliberately left: a nested `expand` sort is also
5133+
* forwarded into the expansion sub-read (`expandRelatedRecords`), and the
5134+
* engine door does fire there — but that sub-read sits inside a pre-existing
5135+
* graceful-degradation `catch` that swallows EVERY expand failure and
5136+
* retains the raw foreign keys. So that one path improves from silent to
5137+
* OBSERVABLE (a warning carrying the field and the remedy) rather than
5138+
* becoming a refusal. Reversing that backstop is the #3821-family swallow —
5139+
* a separate decision on all expand failure modes, not a rider on this one.
5140+
*
5141+
* This gate is UNCHANGED and still the first door: it keeps the `param` name
5142+
* in the message (which the engine cannot know) and the `unknown` >
5143+
* `dotted` > unmaterializable precedence. The engine door deliberately
5144+
* judges only the third verdict — see its docblock for why it does not
5145+
* inherit the other two.
51255146
*/
51265147
private assertSortFieldsExist(object: string, orderBy: ReadonlyArray<{ field: string }>, param: string): void {
51275148
if (orderBy.length === 0) return;
@@ -5239,9 +5260,24 @@ export class ObjectStackProtocolImplementation implements
52395260
* `?status=<typo>` is a 400 and `?select=<typo>` is not, on one endpoint,
52405261
* about the same field map.
52415262
*
5242-
* The engine's tolerance is untouched: it guards INTERNAL callers (hooks,
5243-
* flows, expand sub-reads, registry-less hosts) that never pass through
5244-
* this ingress, exactly like the object-existence gate above.
5263+
* The engine's tolerance on THIS axis is untouched: it guards INTERNAL
5264+
* callers (hooks, flows, expand sub-reads, registry-less hosts) that never
5265+
* pass through this ingress, exactly like the object-existence gate above.
5266+
* An unknown projection name is dropped and the projection falls back to
5267+
* `*`, so the engine still over-returns rather than throwing.
5268+
*
5269+
* [#7095] That tolerance is PER-AXIS, and this docblock used to be read as
5270+
* a statement about the engine in general — it is not one any more, so the
5271+
* limit is written here rather than left to be inferred. On the SORT axis
5272+
* the engine now REFUSES an ORDER BY it cannot materialise
5273+
* (`assertOrderByIsMaterializable`, `@objectstack/objectql`), because the
5274+
* two axes fail differently: a dropped projection name returns MORE than
5275+
* asked (every column, inspectable in the response), while a dropped sort
5276+
* returns the right rows in an order the response cannot be distinguished
5277+
* from a satisfied one — and with `limit`, an arbitrary page of them. The
5278+
* #7095 sweep found no in-tree internal caller relying on the sort drop, so
5279+
* narrowing it cost no caller anything; nothing equivalent has been measured
5280+
* for the projection axis, and this sentence is not a licence to assume it.
52455281
*
52465282
* [#4196] It also owns the projection's SHAPE, which is a different
52475283
* question from its names and is answered first — see below.

0 commit comments

Comments
 (0)