fix(spec,runtime): ActionEngineFacade.find takes the engine query envelope, not a bare filter

.changeset/15124-action-engine-facade-find-query-envelope.md

@@ -0,0 +1,93 @@
+---
+'@objectstack/spec': minor
+'@objectstack/runtime': minor
+---
+
+**BREAKING for action handlers** — `ActionEngineFacade.find` takes the engine's query ENVELOPE; the bare-filter parameter shape is withdrawn (#15124)
+
+Clause-②: yes (narrowing)
+
+`ctx.engine.find(object, query)` now takes `EngineQueryOptions` — the same
+options bag `IDataEngine.find` and ObjectQL's own `engine.find` take, named by
+identity rather than restated. **One platform, one query shape.**
+
+### Migration — FROM → TO
+
+| You wrote | Write instead |
+| --- | --- |
+| `ctx.engine.find('task', { status: 'open' })` | `ctx.engine.find('task', { where: { status: 'open' } })` |
+| `ctx.engine.find('task', { amount: { $gt: 100 } })` | `ctx.engine.find('task', { where: { amount: { $gt: 100 } } })` |
+| `ctx.engine.find('task', {})` | unchanged — an empty envelope is still the unfiltered read |
+
+The rewrite is lossless and mechanical: the filter moves under `where`, verbatim.
+`tsc --noEmit` over your handlers finds every unmigrated call — see below.
+
+### Why the shape was withdrawn rather than the bar closed
+
+Until now this parameter was the `where` HALF of a query while every other
+`find` on the platform took the whole envelope, and the runtime wrapped what it
+was given. That made the most natural spelling the wrong one, silently: an
+author who passed the engine's own envelope reached the engine as
+`{ where: { where: … } }` — a filter on a field named `where` — which matches no
+row and resolves to `[]` with **no error at all**. A handler that made the
+mistake ran to completion over zero rows for as long as it shipped, and its own
+hand-written test double, written to the same belief, passed every assertion.
+Because an empty `{}` skipped the wrap, one unfiltered read kept working under
+either belief, so a dead handler looked partially alive.
+
+Refusing `where` at the top level instead — intersecting the old parameter with
+`{ where?: never }` — was rejected: it asserts a vocabulary fact the spec
+declares nowhere, reserving the field name `where` across every customer's data
+model to buy one parameter's compile-time check. Aligning the parameter removes
+the ambiguity at its root and reserves nothing.
+
+### What the new declaration refuses, measured
+
+If your handler is typed with the published `ActionHandlerContext`, a bare filter
+no longer type-checks on **either** path you can reach it by:
+
+- an object literal (`{ status: 'completed' }`) fails the excess-property check —
+  a field name is not an envelope key;
+- a filter held in a `FilterCondition` variable fails **TS2559** — every envelope
+  key is optional, so a bag of field names has no property in common with it.
+
+The envelope's own keys are typed too: `where: 'a = b'`, `fields: 'id,subject'`
+and `limit: '50'` are each refused.
+
+**If your handler is NOT typed with it** — a handler in an `objectstack.config.js`
+/ `.mjs`, one annotated with your own copy of the context type, or a `(ctx: any)`
+handler — nothing above reaches you, so the facade refuses the withdrawn shape at
+**runtime** instead, before the engine, with the same prescription:
+
+```
+find('task') was given a key 'status' the query envelope does not carry.
+ctx.engine.find(object, query) takes the engine QUERY ENVELOPE, not a bare
+filter — move the filter under `where`: find(object, { where: { … } }).
+Envelope keys: context, cursor, distinct, expand, fields, limit, offset,
+orderBy, search, searchFields, top, where.
+```
+
+⚠️ **That refusal matters most for a filter whose value is `null`.** The engine's
+own unknown-option check exempts a `null` value, because on an option bag a
+`null` is a withdrawal. On a filter it is the "rows with no X" idiom, so
+`{ deleted_at: null }` would have been dropped unexecuted and the read would have
+widened to **every row** — including the ones you were excluding — with no error
+at all. It is refused instead.
+
+### What this opens
+
+`fields`, `orderBy`, `limit`, `offset` and `expand` are reachable from an action
+handler for the first time — under the old parameter there was nowhere to carry
+them. A caller-supplied `context` is **ignored**: this facade is trusted and
+context-less by design, and the runtime stamps its own elevated
+`ExecutionContext` last. Do not write one — it reads as authorization and is
+none.
+
+### Checking a migrated handler
+
+Do not settle for "it still resolves". A handler that had been passing the
+envelope was returning `[]` on **every** call, so a suite written against the
+mistake passes and the row count is the only witness. Re-run each migrated
+handler against seeded data and assert it returns the rows its filter selects.
+
+<!-- adr-0087: registered action-engine-facade-find-query-envelope -->

content/docs/ui/actions.mdx

@@ -170,19 +170,36 @@ already `completed` — is not stamped, so nothing overwrites the handler's valu
 and nothing strips it either, and the action silently replaces the real
 completion timestamp with "now". That is why the snippet sends `status` alone.
 
-<Callout type="warn">
-**`ctx.engine.find(object, filter)` takes a filter, not a query.** The second
-argument is the `where` half only — `{ status: 'completed' }`, operators
-(`{ amount: { $gt: 100 } }`), `$and` / `$or` / `$not` — and the runtime wraps
-it in `where` itself. Passing an ObjectQL envelope
-(`{ where: { status: 'completed' } }`) raises no error: it becomes
-`{ where: { where: … } }`, matches no row, and returns `[]`. An empty filter
-(`{}`) is passed through unwrapped, so the one unfiltered read works under
-either reading and a handler can look partially alive. The parameter is typed
-`FilterCondition` (`ActionEngineFacade` in `@objectstack/spec/ui`), which
-refuses a primitive or a mistyped `$and` / `$or` / `$not` but still admits
-`where` as a key — the sentence above is the contract, and a hand-written test
-double must honour it too.
+<Callout type="info">
+**`ctx.engine.find(object, query)` takes the engine's query envelope.** The
+second argument is the same options bag `engine.find` takes everywhere else —
+the filter goes under `where`, and `fields`, `orderBy`, `limit`, `offset` and
+`expand` mean what they mean on the engine. One platform, one query shape.
+
+```typescript
+await ctx.engine.find('todo_task', { where: { status: 'completed' } });
+await ctx.engine.find('todo_task', { where: { amount: { $gt: 100 } }, fields: ['id', 'subject'], limit: 50 });
+await ctx.engine.find('todo_task', {});   // the unfiltered read
+```
+
+The parameter is typed `EngineQueryOptions` (`ActionEngineFacade` in
+`@objectstack/spec/ui`), so if you annotate `ctx` with the published
+`ActionHandlerContext` a bare filter is a **compile error** at the call site —
+`{ status: 'completed' }` has nowhere to land, and neither does a filter held in
+a `FilterCondition` variable. A hand-written test double must honour the envelope
+too.
+
+If you do **not** annotate it — a handler in an `objectstack.config.js` /
+`.mjs`, your own copy of the context type, or `(ctx: any)` — the facade refuses
+the bare filter at **runtime** instead, naming the stray key and prescribing the
+same fix. That runtime refusal is what stops `{ deleted_at: null }` from being
+dropped unexecuted and quietly widening the read to every row.
+
+**Upgrading?** This parameter used to be the `where` half on its own, and the
+runtime wrapped it. `find(o, f)` becomes `find(o, { where: f })`; an unfiltered
+`find(o, {})` is unchanged. The old shape had the trap the other way round:
+writing the engine's own envelope produced `{ where: { where: … } }`, which
+matched no row and returned `[]` with no error at all.
 </Callout>
 
 <Callout type="info">

examples/app-todo/src/actions/task.handlers.ts

@@ -38,6 +38,11 @@ import type { ActionHandlerContext } from '@objectstack/spec/ui';
 // written against the published type at all. The declaration now says
 // `string | string[]` (#15117), so the copy is gone and this example
 // type-checks against exactly the types a real app gets.
+//
+// And the copy's `query` bag turned out to be the shape the platform kept:
+// #15124 withdrew the bare-filter parameter and `ctx.engine.find` now takes the
+// ENGINE's query envelope — `find(object, { where: … })`, the same options bag
+// `engine.find` takes anywhere else. One platform, one query shape.
 
 /**
  * Mark a single task as complete.
@@ -106,7 +111,9 @@ export async function massCompleteTasks(ctx: ActionHandlerContext): Promise<void
 /** Delete all completed tasks */
 export async function deleteCompletedTasks(ctx: ActionHandlerContext): Promise<void> {
   const { engine } = ctx;
-  const completed = await engine.find('todo_task', { status: 'completed' });
+  // [#15124] The filter goes under `where` — the second argument is the
+  // engine's query envelope, not the `where` half on its own.
+  const completed = await engine.find('todo_task', { where: { status: 'completed' } });
   const ids = completed.map((r) => r.id as string);
   if (ids.length > 0) {
     await engine.delete('todo_task', ids);
@@ -135,6 +142,7 @@ export async function setReminder(ctx: ActionHandlerContext): Promise<void> {
 /** Export tasks to CSV format */
 export async function exportTasksToCSV(ctx: ActionHandlerContext): Promise<string> {
   const { engine } = ctx;
+  // An EMPTY envelope is still the unfiltered read — unchanged by #15124.
   const tasks = await engine.find('todo_task', {});
   const header = 'subject,status,priority,category,due_date';
   const rows = tasks.map((t) =>

packages/runtime/src/action-body-identity.test.ts

@@ -36,9 +36,12 @@ import { QuickJSScriptRunner } from './sandbox/quickjs-runner.js';
  * `ctx.api` binding is exercised end-to-end.
  */
 function makeSharingEngine(extra: Record<string, unknown> = {}) {
-    const writes: Array<{ op: string; object: string; context: any }> = [];
-    const gate = (op: string, object: string, context: any) => {
-        writes.push({ op, object, context });
+    // [#15124] `where` is recorded as well as `context`, so the "the caller's
+    // predicate must survive" case below can assert the PREDICATE instead of
+    // asserting that an entry exists.
+    const writes: Array<{ op: string; object: string; context: any; where?: unknown }> = [];
+    const gate = (op: string, object: string, context: any, where?: unknown) => {
+        writes.push({ op, object, context, where });
         if (!context?.isSystem && !context?.userId) {
             throw new Error(`FORBIDDEN: insufficient privileges to ${op} ${object}`);
         }
@@ -58,7 +61,7 @@ function makeSharingEngine(extra: Record<string, unknown> = {}) {
             return { ok: true };
         },
         async find(object: string, options?: any) {
-            gate('find', object, options?.context);
+            gate('find', object, options?.context, options?.where);
             // [#16370] The by-id pre-load has to be ANSWERED: an action door now
             // refuses a row-scoped invocation whose caller-scope subject load did
             // not deliver the row, so a rig that answered every read with `[]`
@@ -121,7 +124,9 @@ describe('#3914 — ctx.engine (buildActionEngineFacade)', () => {
         await engine.insert('crm_case', { subject: 'x' });
         await engine.update('crm_case', 'case_1', { status: 'closed' });
         await engine.delete('crm_case', 'case_1');
-        await engine.find('crm_case', { status: 'open' });
+        // [#15124] The envelope, not a bare filter — the withdrawn shape is now
+        // refused by the arm itself, so this call would throw if left as it was.
+        await engine.find('crm_case', { where: { status: 'open' } });
 
         expect(ql.writes.map((w: any) => w.op)).toEqual(['insert', 'update', 'delete', 'find']);
         for (const w of ql.writes) {
@@ -138,10 +143,14 @@ describe('#3914 — ctx.engine (buildActionEngineFacade)', () => {
     it('still passes the caller filter on find (context is additive, not a replacement)', async () => {
         const ql = makeSharingEngine();
         const engine = buildActionEngineFacade(deps, ql, { userId: 'u1' });
-        await engine.find('crm_case', { status: 'open' });
+        await engine.find('crm_case', { where: { status: 'open' } });
         expect(ql.writes[0].context).toMatchObject({ isSystem: true, userId: 'u1' });
-        // the caller's predicate must survive alongside the injected context
-        expect((ql.writes as any)[0]).toBeDefined();
+        // [#15124] The caller's predicate must survive alongside the injected
+        // context — asserted on the PREDICATE. This line used to read
+        // `expect(ql.writes[0]).toBeDefined()`, which is true of any recorded
+        // call whatever the arm did with the filter, so the one thing the case
+        // is named for was the one thing it did not check.
+        expect(ql.writes[0].where).toEqual({ status: 'open' });
     });
 });