Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 76 additions & 0 deletions .changeset/engine-find-formula-orderby-refusal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
---
"@objectstack/objectql": major
"@objectstack/metadata-protocol": patch
"@objectstack/spec": patch
---

<!-- adr-0087: registered engine-find-formula-order-by-refused -->

fix(objectql)!: `engine.find` / `engine.findOne` refuse an ORDER BY they cannot materialise (#7095)

`engine.find()` and `engine.findOne()` are a **public API**, and an `orderBy`
naming a `formula` field — which used to return rows successfully, in an
arbitrary order — now **throws `400 INVALID_SORT`**.

#6994 closed this at the REST ingress (`assertSortFieldsExist`), covering
everything that reaches `findData`: the list route, `POST /data/:object/query`,
the export route and the RPC dispatcher. A caller reaching the engine directly
passed through none of it. Measured on the base of this change, 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)
```

A `formula` value is computed on read, so no driver materialises a column for
it: the ORDER BY reached the driver, found nothing, and the unknown-column
backstop returned the rows unordered under a success — carrying the very values
they were asked to be ordered by. With `limit`, "the latest N" was an arbitrary
N that no amount of inspecting the response could reveal.

- FROM `orderBy: [{ field: '<formula field>' }]` → TO: denormalise the value
onto the object (a stored field, written when the source changes) and sort by
that. This is the same remedy, in the same words, that the REST door has
prescribed since #6924 / #6994 and that the SEARCH axis prescribes since
#6673 — a caller refused at two doors is not sent two different ways.

**`summary` / rollup fields are NOT affected** and still sort in both
directions: they get a real, maintained column. The family this refuses is
`formula`, not "computed" — widening it to the spec's `COMPUTED_VALUE_TYPES`
(the *write* contract) would break two types that work, and a control test pins
that.

**Who was actually reaching this.** The #7095 ruling required the internal-caller
tolerance to survive only behind a pinned internal path, and only if a *measured*
internal call site relied on it. The 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 — a
negative test pins that the public options shape refuses one.

The one **author-reachable** consumer is why leaving this at ingress was not
tenable: a saved report's `query.orderBy` is forwarded verbatim into
`engine.find` by `plugin-reports`, bypassing the ingress gate entirely. A report
authored to sort by a formula field used to run and return an arbitrary order;
it now fails loudly with the remedy in the message.

**One path deliberately does NOT become a refusal.** A nested `expand` sort
raises this same error inside `expandRelatedRecords`, but that sub-read sits in a
pre-existing graceful-degradation `catch` which swallows *every* expand failure
and retains the raw foreign keys. That path therefore moves from **silent** to
**observable** — a warning naming the field and the fix — rather than refusing.
Reversing that backstop is a decision about all expand failure modes (#3821) and
is not ridden in on this change; it is measured and pinned as-is.

**What did NOT change:** the ingress gate is untouched — same message, same
`unknown` > `dotted` > unmaterializable precedence, same `param` name that the
engine cannot know. The engine door judges only the third verdict: unknown and
dotted sort names still reach the driver from a direct call exactly as before,
because refusing those is a posture change on two further axes rather than a
free extension of this one. Reading a formula field, and the projection axis'
`SELECT *` tolerance, are also untouched.
9 changes: 9 additions & 0 deletions docs/protocol-upgrade-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,15 @@ No mechanical rewrite exists, in either direction. The refused values carry no r

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.
- 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.
- **`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
- 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.

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.

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.

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.
- 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.

---

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