From af914ea796d3c0733b2060e5c9f8ccdb64803b76 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 20 Sep 2026 08:50:59 +0000 Subject: [PATCH 1/2] fix(spec): subtract `context` from `ActionEngineFacade.find`'s query envelope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The facade is trusted and mints its own elevated ExecutionContext, which the runtime spreads last — so a caller-supplied `context` was overridden, never honoured, while the parameter type went on declaring it. That is ADR-0049's declared-but-unenforced shape on the one key that carries identity and tenant. Take the remove arm, at the declaration layer only: the parameter is now `Omit`. No runtime behaviour changes — the facade arm still accepts the key from the untyped channel and still overrides it, because refusing an identity key there is a runtime permission change no ruling covers. The asymmetry is recorded on both halves rather than closed. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01LvwGppdonww4zGLWZo5rho --- ...n-engine-facade-find-context-subtracted.md | 40 +++++++++++++ content/docs/ui/actions.mdx | 10 ++++ packages/runtime/src/action-execution.ts | 24 ++++++-- packages/spec/src/ui/action-params.test.ts | 56 ++++++++++++++++++- packages/spec/src/ui/action-params.zod.ts | 48 ++++++++++++---- 5 files changed, 159 insertions(+), 19 deletions(-) create mode 100644 .changeset/19237-action-engine-facade-find-context-subtracted.md diff --git a/.changeset/19237-action-engine-facade-find-context-subtracted.md b/.changeset/19237-action-engine-facade-find-context-subtracted.md new file mode 100644 index 00000000000..b06af620c9d --- /dev/null +++ b/.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` — 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. + + diff --git a/content/docs/ui/actions.mdx b/content/docs/ui/actions.mdx index 56545f994c8..34a200843c7 100644 --- a/content/docs/ui/actions.mdx +++ b/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 diff --git a/packages/runtime/src/action-execution.ts b/packages/runtime/src/action-execution.ts index 769ce082534..d2d6ac40c87 100644 --- a/packages/runtime/src/action-execution.ts +++ b/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): Promise>> { // …and the withdrawn shape is refused HERE, before the engine, so // the untyped channel gets the same answer the type gives diff --git a/packages/spec/src/ui/action-params.test.ts b/packages/spec/src/ui/action-params.test.ts index 402b40ce9c3..b8653b26fb4 100644 --- a/packages/spec/src/ui/action-params.test.ts +++ b/packages/spec/src/ui/action-params.test.ts @@ -414,16 +414,43 @@ type FindQuery = Parameters[1]; // the strict mutual-assignability test, so neither `Record` // 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` 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 diff --git a/packages/spec/src/ui/action-params.zod.ts b/packages/spec/src/ui/action-params.zod.ts index 356a7decf17..3f73de3ae0f 100644 --- a/packages/spec/src/ui/action-params.zod.ts +++ b/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>>; + find(object: string, query: Omit): Promise>>; } /** From aa22b1315bc77c8c8b5b4522585b1e818962117e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 20 Sep 2026 08:58:29 +0000 Subject: [PATCH 2/2] chore(spec): regenerate api-surface-declarations for the narrowed find slot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One shard, one semantic hunk: `find(object, query: EngineQueryOptions)` becomes `find(object, query: Omit)`, plus the member doc that states why. No declaration-emit ORDER churn in the other seven shards — this diff adds no import, so the d.ts chunking does not move. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01LvwGppdonww4zGLWZo5rho --- packages/spec/api-surface-declarations/ui.txt | 41 +++++++++++++++---- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/packages/spec/api-surface-declarations/ui.txt b/packages/spec/api-surface-declarations/ui.txt index e48b030fe5a..d023a441588 100644 --- a/packages/spec/api-surface-declarations/ui.txt +++ b/packages/spec/api-surface-declarations/ui.txt @@ -138,7 +138,9 @@ 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: * @@ -203,19 +205,40 @@ 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>>; + find(object: string, query: Omit): Promise>>; } // ── ActionHandler (type) ──