spec(ui): subtract the unenforced context key from ActionEngineFacade.find's query envelope

.changeset/19237-action-engine-facade-find-context-subtracted.md

@@ -0,0 +1,40 @@
+---
+'@objectstack/spec': minor
+---
+
+spec(ui): `ActionEngineFacade.find` no longer accepts a `context` on its query envelope
+
+Clause-②: no (narrowing)
+
+`ctx.engine.find(object, query)` takes `Omit<EngineQueryOptions, 'context'>` — the
+engine's query envelope with exactly one key subtracted. Every other key is
+unchanged and still read off the engine's own type by reference.
+
+**Why.** The action facade is trusted and context-less by design: the runtime
+stamps its own elevated `ExecutionContext` last, so a caller-supplied `context`
+was overridden, never honoured. The key was nonetheless *declared* on the
+parameter, which made this a declared-but-unenforced key on the one thing
+`context` carries — identity and tenant. A handler could write
+`context: { tenantId: 'org_acme' }`, type-check clean, and get the facade's
+context instead: a read its author believes is tenant-scoped, silently broader
+than intended. ADR-0049 admits enforce or remove; removal is the exit that
+changes no runtime behaviour.
+
+**Migration.** Delete the key. There is nothing to replace it with, because it
+never did anything: a `find` that carried one returned exactly the rows it
+returns without one. To scope a read, put the scope in `where`.
+
+| You wrote | Write instead |
+| --- | --- |
+| `ctx.engine.find('task', { where: { … }, context: { tenantId } })` | `ctx.engine.find('task', { where: { … } })` |
+| `ctx.engine.find('task', { where: { … } })` | unchanged |
+
+`tsc --noEmit` over a consumer's handlers finds every occurrence, because the
+key is now an excess property on a fresh literal. ⚠️ Only where the handler is
+annotated with the published `ActionHandlerContext`: an untyped handler (a JS
+config body, a local copy of the context type, `(ctx: any)`) still passes the
+key and still has it overridden, silently, exactly as before. The runtime arm is
+deliberately unchanged — refusing an identity key there is a runtime behaviour
+change, not a declaration narrowing.
+
+<!-- adr-0087: not-required (already-registered action-engine-facade-find-query-envelope) that entry announces this parameter's shape and already states the rule this diff makes the compiler enforce — "A caller-supplied `context` is ignored: the facade is trusted and stamps its own elevated one." A consumer that followed it has no `context` left to remove, so this narrowing adds no migration step to the ledger. -->

content/docs/ui/actions.mdx

@@ -189,6 +189,16 @@ The parameter is typed `EngineQueryOptions` (`ActionEngineFacade` in
 a `FilterCondition` variable. A hand-written test double must honour the envelope
 too.
 
+**One envelope key is not on this parameter: `context`.** A handler's
+`ctx.engine` is trusted and mints its own elevated `ExecutionContext`, so a
+`context` you write here would be overridden, never honoured — and writing one
+reads as narrowing a query to an identity or a tenant when it does nothing of
+the kind. It is subtracted from the type, so `ctx.engine.find('todo_task', {
+where: { … }, context: { tenantId: 'org_acme' } })` is a compile error rather
+than a silent no-op. To scope a read, put the scope in `where`. (An *untyped*
+handler still passes the key and still has it overridden, with no error — one
+more reason to annotate `ctx`.)
+
 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

packages/runtime/src/action-execution.ts

@@ -1602,15 +1602,31 @@ export function buildActionEngineFacade(_deps: ActionExecutionDeps, ql: any, ec?
         // across every customer's data model to refuse it — one platform, one
         // query shape. The spec member (`ActionEngineFacade.find`,
         // `packages/spec/src/ui/action-params.zod.ts`) now declares
-        // `EngineQueryOptions` by identity, so the handler writes what the
+        // `EngineQueryOptions` minus `context`, so the handler writes what the
         // engine reads and this arm only adds the identity.
         //
         // `context` is spread LAST on purpose: the facade is trusted and
         // context-less by design (#3914, ADR-0096), so the elevated context it
-        // built wins over any `context` a caller put in the envelope. The
-        // envelope admits the key because every engine option bag does; it is
-        // not an authorization the caller gets to choose. Pinned in
+        // built wins over any `context` a caller put in the envelope. It is not
+        // an authorization the caller gets to choose. Pinned in
         // `action-engine-facade-find-envelope.test.ts`.
+        //
+        // ⚠️ [#19237] The TYPE no longer admits the key — the spec member
+        // subtracts it with `Omit`, ADR-0049's remove arm — but THIS ARM IS
+        // UNCHANGED and still accepts it. Two halves, deliberately asymmetric:
+        //
+        //   - a TYPED caller now gets a compile error at the call site, which
+        //     is the whole of the #19237 remedy;
+        //   - an UNTYPED one (a JS config handler, a local copy of the context
+        //     type, `(ctx: any)`) still passes a `context` and still has it
+        //     overridden here, silently, exactly as before.
+        //
+        // Closing the second half means making this arm THROW on an identity
+        // key, which is a runtime permission behaviour change and not a thing a
+        // type narrowing gets to smuggle in. `findEnvelopeKeys()` therefore
+        // still reads `context` off `EngineQueryOptionsSchema` as legal, and
+        // the override — not a refusal — is what the untyped channel gets.
+        // ⛔ Do not "finish the job" here without a ruling that covers it.
         async find(object: string, query?: Record<string, unknown>): Promise<Array<Record<string, unknown>>> {
             // …and the withdrawn shape is refused HERE, before the engine, so
             // the untyped channel gets the same answer the type gives

packages/spec/src/ui/action-params.test.ts

@@ -414,16 +414,43 @@ type FindQuery = Parameters<ActionEngineFacade['find']>[1];
 // the strict mutual-assignability test, so neither `Record<string, unknown>`
 // nor `FilterCondition` — the type this slot carried between #14175 and
 // #15124, and the one it must not drift back to — satisfies it. Reading the
-// engine's published type BY IDENTITY is the whole point of the ruling ("one
+// engine's published type BY REFERENCE is the whole point of the ruling ("one
 // platform, one query shape"): a structural copy of the envelope would pass a
 // weaker pin and then drift the moment the engine's own options grow a key.
 // Exported, as the sibling pins are, so `noUnusedLocals` does not read a type
 // that exists only to be checked as one that is never used.
-export type FindQueryIsEngineQueryOptions = Assert< Eq< FindQuery, EngineQueryOptions > >;
+//
+// #19237 subtracts exactly ONE key from that reference. The pin is written as
+// `Omit<EngineQueryOptions, 'context'>` rather than a spelled-out key list for
+// the same reason the slot is: the envelope's OTHER keys stay by reference, so
+// a key the engine grows tomorrow is reachable from a handler on the same day
+// without touching this line, and the only thing this file asserts about the
+// envelope's content is the subtraction itself.
+export type FindQueryIsEngineQueryOptionsWithoutContext =
+  Assert< Eq< FindQuery, Omit< EngineQueryOptions, 'context' > > >;
+
+// The subtraction, stated as its own fact rather than inferred from the `Eq`
+// above — because the two fail differently and a reader needs to know WHICH
+// moved. `Eq` reds for any drift at all (a re-widening, a re-narrowing, a
+// rename of the type behind the slot); this one reds only when `context`
+// becomes writable again, which is the ADR-0049 regression #19237 closed.
+//
+// ⚠️ This is a TYPE-level pin, deliberately, and not a runtime one. The card's
+// remedy is a pure narrowing of a declaration: the runtime's behaviour is
+// UNCHANGED (`buildActionEngineFacade` still spreads its own context last, and
+// its `assertActionEngineFindEnvelope` still reads `context` off
+// `EngineQueryOptionsSchema` as a legal key for the untyped channel). A
+// runtime pin here would assert a refusal that does not exist and must not:
+// adding one is a runtime permission change, which is not this card's to make.
+// The runtime side of the contract keeps its own pin, unchanged, in
+// `packages/runtime/src/action-engine-facade-find-envelope.test.ts`.
+export type FindQueryCarriesNoContextKey =
+  Assert< Eq< 'context' extends keyof FindQuery ? true : false, false > >;
 
 describe('#15124 — ActionEngineFacade.find takes the engine query envelope, never a bare filter', () => {
   it('types the second parameter as the published `EngineQueryOptions` (the tsc channel)', () => {
-    // The value-level half of `FindQueryIsEngineQueryOptions` above: a literal
+    // The value-level half of `FindQueryIsEngineQueryOptionsWithoutContext`
+    // above: a literal
     // annotated with the slot type, so the runtime run exercises the same
     // declaration the type pin reads.
     const query: FindQuery = { where: { position_code: 'qa_lead', active: true } };
@@ -486,6 +513,29 @@ describe('#15124 — ActionEngineFacade.find takes the engine query envelope, ne
     expect([whereNotFilter, fieldsNotArray, limitNotNumber]).toHaveLength(3);
   });
 
+  it('#19237 REFUSAL PIN — `context` is not an envelope key on THIS facade (ADR-0049 remove arm)', () => {
+    // The key the engine honours and this facade does not. It was declared
+    // here and unenforced between #15124 and #19237: the write below
+    // type-checked, and the runtime stamped the facade's own elevated context
+    // over it with no signal — so an author who wrote `context: { tenantId }`
+    // believing they had NARROWED the read got a broader one.
+    //
+    // `@ts-expect-error` is the whole assertion: if the slot ever re-admits
+    // the key, the directive goes unused and `tsc -p tsconfig.test.json` reds.
+    // @ts-expect-error — `context` is subtracted from this parameter; the facade mints its own and a caller-supplied one is never honoured.
+    const narrowingAttempt: FindQuery = { where: { status: 'open' }, context: { tenantId: 'org_acme' } };
+    // @ts-expect-error — the same on its own, with no other key to carry the literal.
+    const contextAlone: FindQuery = { context: { isSystem: true } };
+
+    // POSITIVE CONTROL, in the same test: the rest of the envelope is
+    // untouched by the subtraction. Without this, a pin that "passes" because
+    // the whole parameter degenerated to `never` would read exactly the same.
+    const rest: FindQuery = { where: { status: 'open' }, fields: ['id'], orderBy: [{ field: 'id', order: 'asc' }], limit: 5, offset: 0 };
+
+    expect([narrowingAttempt, contextAlone]).toHaveLength(2);
+    expect(Object.keys(rest)).toEqual(['where', 'fields', 'orderBy', 'limit', 'offset']);
+  });
+
   it('the refusal survives the VARIABLE path too — not just the object-literal check', () => {
     // The obvious worry about narrowing an all-optional target is that only
     // FRESH object literals get the excess-property check, so a filter reaching

packages/spec/src/ui/action-params.zod.ts

@@ -233,9 +233,10 @@ export function validateActionParams(
  *
  * Two members carry an argument contract the signature alone does not settle,
  * and both state it on the member: `find` takes the engine's own query
- * ENVELOPE — {@link EngineQueryOptions}, by identity, the same type
- * `IDataEngine.find` takes — and the bare-filter parameter shape #14175 chose
- * is withdrawn (#15124); `delete` accepts a single id OR an array of them,
+ * ENVELOPE — {@link EngineQueryOptions} minus its `context` key, the same type
+ * `IDataEngine.find` takes with the one key this facade will not honour
+ * subtracted (#15124, #19237) — and the bare-filter parameter shape #14175
+ * chose is withdrawn (#15124); `delete` accepts a single id OR an array of them,
  * both as declared contract, served one row at a time (#15117). Read those doc
  * comments before writing a handler or a test double against either.
  */
@@ -282,7 +283,9 @@ export interface ActionEngineFacade {
    *
    * `query` is the ENGINE's query envelope — {@link EngineQueryOptions}, the
    * very type `IDataEngine.find` and ObjectQL's own `engine.find` take, named
-   * here by identity rather than restated. The filter goes under `where`, and
+   * here by reference rather than restated, with exactly ONE key subtracted:
+   * `context`, which this facade does not honour (the section at the bottom of
+   * this comment). The filter goes under `where`, and
    * the rest of the envelope (`fields`, `orderBy`, `limit`, `offset`,
    * `expand`, `search`, …) means exactly what it means on the engine:
    *
@@ -347,19 +350,40 @@ export interface ActionEngineFacade {
    * straight through would be dropped unexecuted and the read would widen to
    * EVERY row, silently, to a caller whose next line is often a delete.
    *
-   * ## `context` is the caller's to pass and NOT the caller's to choose
+   * ## `context` is not on this parameter — ADR-0049 enforce-or-remove (#19237)
    *
-   * The envelope carries `context` because every engine option bag does. This
-   * facade is TRUSTED and context-less by design (#2849, ADR-0096): the
-   * runtime stamps its own elevated `ExecutionContext` last, so a
-   * caller-supplied `context` is overridden rather than honoured. Do not write
-   * one — it reads as authorization and is none.
+   * The engine's envelope carries `context` because every engine option bag
+   * does, and on the engine it is honoured: it is where identity and tenant
+   * live. On THIS facade it is not. The facade is TRUSTED and context-less by
+   * design (#2849, ADR-0096) — the runtime stamps its own elevated
+   * `ExecutionContext` last (`buildActionEngineFacade`,
+   * `packages/runtime/src/action-execution.ts`), so a caller-supplied
+   * `context` is overridden, never honoured.
+   *
+   * Between #15124 and #19237 the key was therefore DECLARED here and
+   * unenforced: a handler could write `context: { tenantId: … }`, type-check
+   * clean, and get the facade's context instead — a read the author believes
+   * is tenant-scoped, silently broader than intended. ADR-0049 admits two
+   * exits for a declared-but-unenforced key, enforce or remove, and removal is
+   * the one that changes no runtime behaviour: the key is subtracted from this
+   * parameter with `Omit`, so writing one is a compile error at the call site
+   * instead of a no-op at runtime.
+   *
+   * ⚠️ The RUNTIME still tolerates the key, deliberately and unchanged. The
+   * facade's arm reads its legal key set off `EngineQueryOptionsSchema`, which
+   * still declares `context`, so an UNTYPED caller (a handler in an
+   * `objectstack.config.js` / `.mjs`, a local copy of the context type, a
+   * `(ctx: any)` handler) still passes one and still has it overridden rather
+   * than refused. Refusing it there would be a new runtime refusal on an
+   * identity key — a behaviour change, out of this card's scope by ruling, and
+   * the asymmetry is recorded rather than silently closed.
    *
    * Every clause above is pinned in `action-params.test.ts`, and the
-   * pass-through is pinned against the runtime in
+   * pass-through — including the untyped channel's surviving tolerance — is
+   * pinned against the runtime in
    * `packages/runtime/src/action-engine-facade-find-envelope.test.ts`.
    */
-  find(object: string, query: EngineQueryOptions): Promise<Array<Record<string, unknown>>>;
+  find(object: string, query: Omit<EngineQueryOptions, 'context'>): Promise<Array<Record<string, unknown>>>;
 }
 
 /**