From 1ff0e37ed13583026a56b0eab6693c336cd6590e Mon Sep 17 00:00:00 2001 From: "claude[bot]" Date: Mon, 10 Aug 2026 07:23:11 +0000 Subject: [PATCH 1/2] fix(objectql)!: `engine.find`/`findOne` refuse an unmaterializable formula ORDER BY (#7095) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #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: , order: 'asc' }] }) -> C A E B D engine.find(o, { orderBy: [{ field: , 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 Claude-Session: https://claude.ai/code/session_01HJgGahRqaYPRJ2oKmk9Czc --- .../engine-find-formula-orderby-refusal.md | 76 ++++++++ docs/protocol-upgrade-guide.md | 9 + packages/metadata-protocol/src/protocol.ts | 62 ++++-- packages/objectql/src/engine.ts | 110 +++++++++++ .../src/query-expression-conformance.test.ts | 178 ++++++++++++++++-- packages/spec/spec-changes.json | 14 ++ packages/spec/src/migrations/registry.ts | 61 ++++++ 7 files changed, 478 insertions(+), 32 deletions(-) create mode 100644 .changeset/engine-find-formula-orderby-refusal.md diff --git a/.changeset/engine-find-formula-orderby-refusal.md b/.changeset/engine-find-formula-orderby-refusal.md new file mode 100644 index 0000000000..615a89e87d --- /dev/null +++ b/.changeset/engine-find-formula-orderby-refusal.md @@ -0,0 +1,76 @@ +--- +"@objectstack/objectql": major +"@objectstack/metadata-protocol": patch +"@objectstack/spec": patch +--- + + + +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: , order: 'asc' }] }) -> C A E B D +engine.find(o, { orderBy: [{ field: , 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: '' }]` → 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. diff --git a/docs/protocol-upgrade-guide.md b/docs/protocol-upgrade-guide.md index 341a88b3fa..b0bcb6db01 100644 --- a/docs/protocol-upgrade-guide.md +++ b/docs/protocol-upgrade-guide.md @@ -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. --- diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 9763b804f4..eb92dcdb46 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -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; @@ -5071,9 +5092,24 @@ export class ObjectStackProtocolImplementation implements * `?status=` is a 400 and `?select=` 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. diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index f43be49db7..eb55037061 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -663,6 +663,106 @@ function planFormulaProjection( return { plan }; } +/** + * [#7095] ORDER BY a field whose value is computed on read — refused HERE, on + * the engine's own public boundary, and no longer only at the REST ingress. + * + * #6994 closed this at `assertSortFieldsExist` (`400 INVALID_SORT`), which + * covers everything reaching `findData`: the REST list route, + * `POST /data/:object/query`, the export route and the RPC dispatcher. It could + * not cover a caller that reaches {@link ObjectQL.find} / {@link + * ObjectQL.findOne} DIRECTLY, and that half was not hypothetical — measured on + * this file's base (real `ObjectQL`, a driver that really sorts): + * + * ``` + * engine.find(o, { orderBy: [{ field: , order: 'asc' }] }) -> C A E B D (insertion order) + * engine.find(o, { orderBy: [{ field: , order: 'desc' }] }) -> C A E B D (byte-identical) + * ``` + * + * `asc` and `desc` coming back identical is what makes it a DROPPED sort rather + * than a coincidence, and the rows carrying the very values they were asked to + * be ordered by is what makes it invisible. `planFormulaProjection` above drops + * the virtual NAME from the projection (it must — the driver has no column and + * `SELECT sort_key` fails as "no such column"); nothing did the equivalent for + * the sort, so the ORDER BY reached the driver, found nothing, and the #3821 + * unknown-column backstop returned the rows unordered under a success. + * + * WHY A REFUSAL AND NOT AN OBSERVABLE DROP (the card offered both): ruled + * 2026-08-10 on #7095 — an ORDER BY the engine cannot apply is a 4xx with + * guidance prose, never a silent drop, the same direction as the analytics + * dataset refusal envelope and #6924's 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 found none, so + * there is no internal path and no option to opt back into the drop. That sweep + * is recorded in the changeset — if you are adding a caller that WANTS the old + * behaviour, the answer is a stored field, not a flag. + * + * ⚠️ Post-hoc sorting after {@link applyFormulaPlan} evaluates the formulas is + * a TRAP, written down here because it looks like the generous fix: `driver.find` + * has already applied `limit`/`offset`, so re-sorting reorders an ARBITRARY + * PAGE. It would pass every small-result-set test and be wrong the moment + * pagination is involved. + * + * SCOPE — deliberately the third verdict only. `unknown` and `dotted` names are + * NOT judged here: the ingress gate's precedence is `unknown` > `dotted` > + * unmaterializable (#4226 / #4256 / #6994), and the engine has always tolerated + * an unknown projection name by design (the `SELECT *` tolerance a few lines + * below). Widening this door to those two is a separate posture change on two + * more axes, not a free extension of this one — so a dotted path keeps reaching + * the driver exactly as before, including one whose head is a formula field. + * + * A registry-less host (`schema` undefined) returns early, exactly as the + * ingress gate returns early when `resolveQueryFields` cannot answer: a door + * that cannot see the field map must not invent a verdict about it. + */ +function assertOrderByIsMaterializable( + object: string, + operation: 'find' | 'findOne', + schema: any, + orderBy: unknown, +): void { + if (!Array.isArray(orderBy) || orderBy.length === 0) return; + if (!schema?.fields) return; + const names = orderBy.map((node) => + typeof node === 'string' ? node : String((node as any)?.field ?? '')); + const unmaterialized = names.filter((f) => + f !== '' && !f.includes('.') && (schema.fields as any)[f]?.type === 'formula'); + if (unmaterialized.length === 0) return; + const first = unmaterialized[0]; + const type = String((schema.fields as any)[first]?.type); + const err: any = new Error( + `ObjectQL.${operation}('${object}') sorts by '${first}', a ${type} field on '${object}' — ` + + `a ${type} value is computed on read, so no driver materialises a column to order by` + + (unmaterialized.length > 1 ? ` (also: ${unmaterialized.slice(1).join(', ')})` : '') + + '. It was not applied, and an unapplied sort returns the rows in an arbitrary order — ' + + "which 'limit'/'offset' then slices into an arbitrary page." + // Deliberately the SAME remedy, in the same words, as the ingress door's + // formula and dotted refusals (#6994, #6924) and #6673's SEARCH-axis + // correction. One vocabulary across the doors: a caller refused at the REST + // boundary and a caller refused here must not be sent two different ways. + // `query-expression-conformance.test.ts` pins the three wordings as EQUAL + // rather than each separately, because separate wordings is exactly how + // #4256 and #6673 drifted apart in the first place. + + ` Denormalise the value onto '${object}' (a stored field, written when the` + + ' source changes) and sort by that. A formula field is virtual: with no' + + ' column behind it the ORDER BY reaches the driver, finds nothing, and is' + + ' dropped — the arbitrary order this refusal replaces.', + ); + // `INVALID_SORT`, not a new code, and 400 rather than 500: the ingress door + // reasoned that one condition — "this sort was not applied as written" — + // keeps ONE wire code however the caller reached it, and a caller reaching + // the engine directly has not stopped being that condition. A host that + // surfaces engine errors over HTTP therefore answers the same envelope on + // both doors instead of turning the direct path into an unhandled 500. + err.status = 400; + err.code = 'INVALID_SORT'; + err.field = first; + err.fields = unmaterialized; + err.object = object; + throw err; +} + /** * Evaluate formula virtual fields against the raw rows a driver handed back — * the read path (`find` / `findOne`) and, since #5504, the write path's @@ -6262,6 +6362,11 @@ export class ObjectQL implements IObjectQLEngine { const _findSchema = this._registry.getObject(object); this.expandSearchOnAst(ast, _findSchema); + // [#7095] Before the projection is planned and before anything is handed to + // a driver: an ORDER BY this engine cannot materialise is refused, not + // dropped. `fillQueryAstDefaults` has already normalised `orderBy` into + // `SortNode[]`, so the names read here are the ones the driver would get. + assertOrderByIsMaterializable(object, 'find', _findSchema, ast.orderBy); const _findFormula = planFormulaProjection(_findSchema, ast.fields); if (_findFormula.projected) ast.fields = _findFormula.projected; @@ -6408,6 +6513,11 @@ export class ObjectQL implements IObjectQLEngine { // counts as the predicate it is (#4419). this.expandSearchOnAst(ast, _findOneSchema); this.requireFindOnePredicate(objectName, ast); + // [#7095] Same refusal as `find`, and it matters MORE here: `findOne` + // applies `limit: 1`, so `orderBy` is the whole of "which record" — a + // dropped sort does not merely reorder the answer, it returns a DIFFERENT + // record, and the one it returns looks exactly as legitimate. + assertOrderByIsMaterializable(objectName, 'findOne', _findOneSchema, ast.orderBy); const _findOneFormula = planFormulaProjection(_findOneSchema, ast.fields); if (_findOneFormula.projected) ast.fields = _findOneFormula.projected; diff --git a/packages/objectql/src/query-expression-conformance.test.ts b/packages/objectql/src/query-expression-conformance.test.ts index 24aa6b3480..333bc574f5 100644 --- a/packages/objectql/src/query-expression-conformance.test.ts +++ b/packages/objectql/src/query-expression-conformance.test.ts @@ -629,10 +629,17 @@ describe('#4226 — sort / select / expand on the list path (real ObjectQL engin expect(err.message).not.toMatch(/formula or rollup/); }); - it('the two refusals agree word-for-word on the remedy', async () => { + it('the three refusals agree word-for-word on the remedy', async () => { // Pins the AGREEMENT itself rather than each wording separately: this - // goes red if either door's remedy is reworded without the other, which + // goes red if any door's remedy is reworded without the others, which // is exactly how #4256 and #6673 drifted apart in the first place. + // + // [#7095] Three doors now, not two: the engine's own boundary emits the + // same sentence as the two ingress verdicts. It is the whole reason the + // engine door duplicates the prose instead of importing it — + // `metadata-protocol` is assembled FROM an engine, so the engine cannot + // import from it without inverting the layering. This pin is what keeps + // the duplication honest. const remedy = /Denormalise the value onto 'showcase_task' \(a stored field, written when the source changes\) and sort by that\./; const dotted: any = await protocol .findData({ object: 'showcase_task', query: { sort: 'project_id.name' } }) @@ -640,8 +647,12 @@ describe('#4226 — sort / select / expand on the list path (real ObjectQL engin const formula: any = await protocol .findData({ object: 'showcase_task', query: { sort: 'sort_key' } }) .then(() => null, (e: unknown) => e); + const direct: any = await engine + .find('showcase_task', { orderBy: [{ field: 'sort_key', order: 'asc' }] }) + .then(() => null, (e: unknown) => e); expect(dotted.message).toMatch(remedy); expect(formula.message).toMatch(remedy); + expect(direct.message).toMatch(remedy); }); it.each([ @@ -657,24 +668,153 @@ describe('#4226 — sort / select / expand on the list path (real ObjectQL engin .rejects.toMatchObject({ status: 400, code: 'INVALID_SORT', field }); }); - it('RECORD OF A KNOWN HOLE — `engine.find` still drops a formula sort silently (#6994 is ingress-only)', async () => { - // NOT a defence of this behaviour: a pin on the half the ingress gate - // cannot reach, so it is measured rather than assumed closed. Internal - // callers (hooks, flows, reports, expand sub-reads) never pass through - // `findData`, so they still get the 200-in-arbitrary-order this axis - // refuses at the door. Closing it means deciding whether `engine.find` - // REFUSES or keeps its documented internal-caller tolerance — an - // engine-core contract decision, tracked separately. + // ───────────────────────────────────────────────────────────── + // [#7095] THE HOLE THIS FILE USED TO RECORD, now closed. + // + // Until #7095 the test below was `RECORD OF A KNOWN HOLE`: it asserted that + // `engine.find` returned INSERTION_ORDER for both `asc` and `desc`, and + // said in as many words that it should go red the day the engine grew this + // refusal. This is that day, and this is that test, inverted. + // + // Ruled 2026-08-10 on #7095: an ORDER BY the engine cannot materialise is a + // 4xx with guidance prose at the PUBLIC boundary, never a silent drop. The + // internal-caller 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 (see the changeset), so no internal path exists and the pins + // below include a negative one saying so. + // ───────────────────────────────────────────────────────────── + + it('a real column still sorts through `engine.find` — the control this refusal needs', async () => { + // FIRST, because every rejection pin under it is vacuous against an + // engine whose sort is simply broken. The direct path must still SORT. + const asc = await engine.find('showcase_task', { orderBy: [{ field: 'title', order: 'asc' }] }); + const desc = await engine.find('showcase_task', { orderBy: [{ field: 'title', order: 'desc' }] }); + expect(asc.map((r: any) => r.title)).toEqual(['A', 'B', 'C', 'D', 'E']); + expect(desc.map((r: any) => r.title)).toEqual(['E', 'D', 'C', 'B', 'A']); + // And a `summary` field still sorts here too — the family is `formula`, + // not "computed". This is what goes red if the engine door is ever + // widened to the spec's `COMPUTED_VALUE_TYPES` (the WRITE contract). + const bySummary = await engine.find('showcase_task', { orderBy: [{ field: 'subtask_total', order: 'asc' }] }); + expect(bySummary.map((r: any) => r.title)).toEqual(['E', 'C', 'D', 'B', 'A']); + }); + + it.each([ + ['ascending', [{ field: 'sort_key', order: 'asc' }]], + ['descending', [{ field: 'sort_key', order: 'desc' }]], + ['second of two', [{ field: 'title', order: 'asc' }, { field: 'sort_key', order: 'asc' }]], + ])('`engine.find` REFUSES a formula ORDER BY instead of dropping it — %s', async (_label, orderBy) => { + // The public boundary, reached directly — no protocol, no ingress gate. + // This is the exact call that answered 200-in-insertion-order before + // #7095, for both directions, byte-identically. + await expect(engine.find('showcase_task', { orderBy: orderBy as any })) + .rejects.toMatchObject({ + status: 400, + code: 'INVALID_SORT', + field: 'sort_key', + object: 'showcase_task', + }); + }); + + it('`engine.findOne` refuses it too — there `orderBy` decides WHICH record', async () => { + // Worse than find's arbitrary order: `findOne` applies `limit: 1`, so a + // dropped sort returns a DIFFERENT record, and it looks as legitimate + // as the right one. `where` is present so this is the sort verdict and + // not `requireFindOnePredicate` answering first. + await expect(engine.findOne('showcase_task', { + where: { status: 'open' }, + orderBy: [{ field: 'sort_key', order: 'desc' }], + })).rejects.toMatchObject({ + status: 400, + code: 'INVALID_SORT', + field: 'sort_key', + object: 'showcase_task', + }); + }); + + it('the engine refusal names the field, the type and the fix — guidance prose, not just a throw', async () => { + const err: any = await engine + .find('showcase_task', { orderBy: [{ field: 'sort_key', order: 'asc' }] }) + .then(() => null, (e: unknown) => e); + expect(err).toBeTruthy(); + // ADR-0112 envelope — a rejection case asserts code AND status. + expect(err.status).toBe(400); + expect(err.code).toBe('INVALID_SORT'); + // It must name the entry point, or a caller who never wrote a query + // parameter cannot tell which door refused them. + expect(err.message).toMatch(/ObjectQL\.find\('showcase_task'\)/); + expect(err.message).toMatch(/a formula field on 'showcase_task'/); + expect(err.message).toMatch(/computed on read/); + // ...and it must never prescribe the thing it is refusing. + expect(err.message).not.toMatch(/formula or rollup/); + }); + + it('an `expand` sub-read raises the refusal, which the expand backstop downgrades to a warning', async () => { + // MEASURED, not assumed, and it is the one place the refusal does not + // reach the caller as a 4xx. A nested `expand` sort is forwarded into + // `expandRelatedRecords`' own `this.find(...)`, which never passes + // through `assertSortFieldsExist` — so the engine door IS what fires + // there. But that sub-read sits inside a pre-existing graceful- + // degradation `catch` ("if expand fails, keep original IDs") which + // swallows EVERY expand failure, this one included: + // + // WARN Failed to expand relationship field; retaining foreign key IDs + // { field: 'parent_id', error: "ObjectQL.find('showcase_task') sorts by + // 'sort_key', a formula field … Denormalise the value onto … " } // - // When that lands, this test SHOULD go red. Update it then; do not - // reach for it as evidence that the direct path is fine. - const asc = await engine.find('showcase_task', { orderBy: [{ field: 'sort_key', order: 'asc' }] }); - const desc = await engine.find('showcase_task', { orderBy: [{ field: 'sort_key', order: 'desc' }] }); - expect(asc.map((r: any) => r.title)).toEqual(INSERTION_ORDER); - // Direction-blind, and the rows carry the values they were meant to be - // ordered by — the exact signature from the issue's transcript. - expect(desc.map((r: any) => r.title)).toEqual(asc.map((r: any) => r.title)); - expect(asc.map((r: any) => r.sort_key)).toEqual(INSERTION_ORDER); + // So the outcome here improves from SILENT (a wrongly-ordered expansion, + // no signal at all) to OBSERVABLE (a warning carrying the field name and + // the fix) — but it is not a refusal, and this pin says so rather than + // implying #7095 closed it. Reversing that catch is the #3821-family + // swallow, a separate contract decision on every expand failure mode, + // deliberately NOT ridden in on this card. Tracked as follow-up. + const rows: any = await engine.find('showcase_task', { + expand: { parent_id: { orderBy: [{ field: 'sort_key', order: 'asc' }] } }, + } as any); + // The call succeeds and the FK ids are retained UNEXPANDED — that is + // the backstop's contract, and what makes this observable-not-refused. + expect(rows).toHaveLength(5); + expect(rows.filter((r: any) => typeof r.parent_id === 'object' && r.parent_id !== null)).toHaveLength(0); + expect(rows.some((r: any) => r.parent_id === 't_A')).toBe(true); + // CONTROL — a real column in the same nested position really does + // expand, so the assertion above is about the refusal and not about + // expand being broken for every sort. + const ok: any = await engine.find('showcase_task', { + expand: { parent_id: { orderBy: [{ field: 'title', order: 'asc' }] } }, + } as any); + expect(ok.some((r: any) => typeof r.parent_id === 'object' && r.parent_id !== null)).toBe(true); + }); + + it('NEGATIVE PIN — no internal path exists to opt back into the drop', async () => { + // §The ruling allowed a pinned INTERNAL path only if a measured internal + // call site relied on the tolerance. The #7095 sweep found none, so no + // such path shipped — and this pin is what keeps one from being added + // quietly on the PUBLIC options shape, which the ruling forbids outright. + // + // `rejectUnknownEngineOptions` refuses any option key not in + // ENGINE_FIND_OPTION_KEYS, so a flag smuggled onto the public bag is a + // refusal about the OPTION, never a tolerated sort. (That refusal is a + // plain `Error` with no `status` — it is #4371's option-shape door, not + // the ADR-0112 wire envelope — so this asserts the message, which is + // what a caller reaching for such a flag would actually be told.) + for (const smuggled of ['allowUnmaterializedSort', 'internal', '__internal', 'tolerateDroppedSort']) { + await expect(engine.find('showcase_task', { + orderBy: [{ field: 'sort_key', order: 'asc' }], + [smuggled]: true, + } as any)).rejects.toThrow(new RegExp(`does not recognise option.*'${smuggled}'`)); + } + // And the tolerance really is gone rather than merely unreachable: the + // shape the old hole answered 200 for now throws. + await expect(engine.find('showcase_task', { orderBy: [{ field: 'sort_key', order: 'asc' }] })) + .rejects.toMatchObject({ code: 'INVALID_SORT' }); + }); + + it('a formula field is still SELECTABLE and still computed — only the ORDER BY is refused', async () => { + // The blast radius, pinned: #7095 narrows one axis. Reading a formula + // field, and the projection tolerance the ingress docblock describes, + // are untouched — a refusal that also stopped formulas being returned + // would be a much larger change wearing this one's clothes. + const rows = await engine.find('showcase_task', { fields: ['id', 'title', 'sort_key'] }); + expect(rows.map((r: any) => r.sort_key).sort()).toEqual(['A', 'B', 'C', 'D', 'E']); }); // ───────────────────────────────────────────────────────────── diff --git a/packages/spec/spec-changes.json b/packages/spec/spec-changes.json index aa55208261..eda4fe8239 100644 --- a/packages/spec/spec-changes.json +++ b/packages/spec/spec-changes.json @@ -798,6 +798,13 @@ "migrationId": "hook-register-empty-object-target-refused", "toMajor": 17, "rationale": "#4281 ruled that an empty hook target is not \"no target\" and closed the shape at the two METADATA doors — `HookSchema.object`'s refine and `hook-binder.ts`'s `normalizeObjects`. `engine.registerHook`, the CODE door, goes through neither, so all three spellings still registered, each producing a defect the author did not write: `''` is FALSY, so the allow face was skipped entirely and the entry became a GLOBAL hook (#4281's headline failure mode — blank intent taking the broadest possible blast radius); `[]` and `['']` are truthy but admit no object name, so the entry could never fire. #5928 then added the `excludeObjects` face, which brought a fourth shape reached by arithmetic rather than by one bad name: an `object` list every member of which is also excluded admits nothing, so that entry can never fire either. All four are ADR-0078 silently-inert declarations, and all four are now refused at REGISTRATION.\n\nNo mechanical rewrite exists, in either direction. The refused values carry no recoverable intent — `object: ''` could have meant `'*'` (what it actually did) or a specific object name the author forgot to fill in, and those are opposite registrations; choosing between them is a judgment the chain cannot make. Nor could the MATCHING read be changed instead: teaching the matcher that `''` is an unmatchable name would silently convert a hook firing on every object into one firing on none — the same class of defect pointing the other way, which is why #5928 declined to do it in passing.\n\nThis 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." + }, + { + "surface": "engine.find(object, { orderBy }) and engine.findOne(object, { orderBy }) naming a `formula` field — the direct engine path, not the REST ingress", + "replacement": "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", + "migrationId": "engine-find-formula-order-by-refused", + "toMajor": 17, + "rationale": "#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.\n\nRuled 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.\n\nThis 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.\n\nONE 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." } ], "removed": [] @@ -1655,6 +1662,13 @@ "migrationId": "hook-register-empty-object-target-refused", "toMajor": 17, "rationale": "#4281 ruled that an empty hook target is not \"no target\" and closed the shape at the two METADATA doors — `HookSchema.object`'s refine and `hook-binder.ts`'s `normalizeObjects`. `engine.registerHook`, the CODE door, goes through neither, so all three spellings still registered, each producing a defect the author did not write: `''` is FALSY, so the allow face was skipped entirely and the entry became a GLOBAL hook (#4281's headline failure mode — blank intent taking the broadest possible blast radius); `[]` and `['']` are truthy but admit no object name, so the entry could never fire. #5928 then added the `excludeObjects` face, which brought a fourth shape reached by arithmetic rather than by one bad name: an `object` list every member of which is also excluded admits nothing, so that entry can never fire either. All four are ADR-0078 silently-inert declarations, and all four are now refused at REGISTRATION.\n\nNo mechanical rewrite exists, in either direction. The refused values carry no recoverable intent — `object: ''` could have meant `'*'` (what it actually did) or a specific object name the author forgot to fill in, and those are opposite registrations; choosing between them is a judgment the chain cannot make. Nor could the MATCHING read be changed instead: teaching the matcher that `''` is an unmatchable name would silently convert a hook firing on every object into one firing on none — the same class of defect pointing the other way, which is why #5928 declined to do it in passing.\n\nThis 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." + }, + { + "surface": "engine.find(object, { orderBy }) and engine.findOne(object, { orderBy }) naming a `formula` field — the direct engine path, not the REST ingress", + "replacement": "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", + "migrationId": "engine-find-formula-order-by-refused", + "toMajor": 17, + "rationale": "#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.\n\nRuled 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.\n\nThis 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.\n\nONE 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." } ], "removed": [] diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index cbe7649c17..9a94deaa6d 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -3405,6 +3405,67 @@ const step17: MigrationStep = { + '"[ObjectQL] Hook ... declares an empty `object` target" throw and no ' + '"[record-change] ... not bound" warning naming a flow you expect to fire.', }, + { + id: 'engine-find-formula-order-by-refused', + surface: + 'engine.find(object, { orderBy }) and engine.findOne(object, { orderBy }) naming a ' + + '`formula` field — the direct engine path, not the REST ingress', + replacement: + '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', + reason: + '#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.\n\n' + + '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.\n\n' + + '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.\n\n' + + '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.', + acceptanceCriteria: + '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.', + }, ], }; From dfa250207f07d80065297b4100b839ac2d9b8b5b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 10 Aug 2026 09:04:23 +0000 Subject: [PATCH 2/2] test(objectql): type the #7095 query-options sites instead of erasing them to `any` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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]>`, 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`, 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 Claude-Session: https://claude.ai/code/session_01HJgGahRqaYPRJ2oKmk9Czc --- .../src/query-expression-conformance.test.ts | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/packages/objectql/src/query-expression-conformance.test.ts b/packages/objectql/src/query-expression-conformance.test.ts index 333bc574f5..9ea5b47d35 100644 --- a/packages/objectql/src/query-expression-conformance.test.ts +++ b/packages/objectql/src/query-expression-conformance.test.ts @@ -52,6 +52,7 @@ import { describe, it, expect, beforeEach } from 'vitest'; import { ObjectStackProtocolImplementation } from '@objectstack/metadata-protocol'; +import type { EngineQueryOptions } from '@objectstack/spec/data'; import { ObjectQL } from './engine.js'; const projectObject = { @@ -698,15 +699,23 @@ describe('#4226 — sort / select / expand on the list path (real ObjectQL engin expect(bySummary.map((r: any) => r.title)).toEqual(['E', 'C', 'D', 'B', 'A']); }); - it.each([ + // Typed as the contract rather than asserted through it: these three sorts + // are perfectly well-formed `SortNode[]` — it is the FIELD they name that + // the engine refuses, not their shape. An `as any` here would erase the one + // channel that enforces `{ field, order }` on a direct engine call + // (`query-options/no-any-erasure`, #4674/#4918), and would have hidden the + // very `direction`-vs-`order` mistake that rule exists to catch. + const REFUSED_SORTS: Array<[string, NonNullable]> = [ ['ascending', [{ field: 'sort_key', order: 'asc' }]], ['descending', [{ field: 'sort_key', order: 'desc' }]], ['second of two', [{ field: 'title', order: 'asc' }, { field: 'sort_key', order: 'asc' }]], - ])('`engine.find` REFUSES a formula ORDER BY instead of dropping it — %s', async (_label, orderBy) => { + ]; + + it.each(REFUSED_SORTS)('`engine.find` REFUSES a formula ORDER BY instead of dropping it — %s', async (_label, orderBy) => { // The public boundary, reached directly — no protocol, no ingress gate. // This is the exact call that answered 200-in-insertion-order before // #7095, for both directions, byte-identically. - await expect(engine.find('showcase_task', { orderBy: orderBy as any })) + await expect(engine.find('showcase_task', { orderBy })) .rejects.toMatchObject({ status: 400, code: 'INVALID_SORT', @@ -769,7 +778,7 @@ describe('#4226 — sort / select / expand on the list path (real ObjectQL engin // deliberately NOT ridden in on this card. Tracked as follow-up. const rows: any = await engine.find('showcase_task', { expand: { parent_id: { orderBy: [{ field: 'sort_key', order: 'asc' }] } }, - } as any); + }); // The call succeeds and the FK ids are retained UNEXPANDED — that is // the backstop's contract, and what makes this observable-not-refused. expect(rows).toHaveLength(5); @@ -780,7 +789,7 @@ describe('#4226 — sort / select / expand on the list path (real ObjectQL engin // expand being broken for every sort. const ok: any = await engine.find('showcase_task', { expand: { parent_id: { orderBy: [{ field: 'title', order: 'asc' }] } }, - } as any); + }); expect(ok.some((r: any) => typeof r.parent_id === 'object' && r.parent_id !== null)).toBe(true); }); @@ -797,10 +806,15 @@ describe('#4226 — sort / select / expand on the list path (real ObjectQL engin // the ADR-0112 wire envelope — so this asserts the message, which is // what a caller reaching for such a flag would actually be told.) for (const smuggled of ['allowUnmaterializedSort', 'internal', '__internal', 'tolerateDroppedSort']) { + // `as unknown as EngineQueryOptions`, never a bare `as any`: this + // input is DELIBERATELY off-contract — that is the whole subject of + // the assertion — so the cast names the contract being bypassed and + // greps as an intentional act, while the rest of the call stays + // type-checked (#4918's prescription for exactly this case). await expect(engine.find('showcase_task', { orderBy: [{ field: 'sort_key', order: 'asc' }], [smuggled]: true, - } as any)).rejects.toThrow(new RegExp(`does not recognise option.*'${smuggled}'`)); + } as unknown as EngineQueryOptions)).rejects.toThrow(new RegExp(`does not recognise option.*'${smuggled}'`)); } // And the tolerance really is gone rather than merely unreachable: the // shape the old hole answered 200 for now throws.