You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
spec(ui): subtract the unenforced context key from ActionEngineFacade.find's query envelope (#19315)
Fixes#19237
Clause-②: no
⚠️ Notation: TypeScript angle brackets are written with PARENTHESES
throughout this body — `Omit(EngineQueryOptions, 'context')` means the
`Omit` utility type. The platform rewrites tag-shaped fragments in a
body, and a fence does not protect them, so the real spelling lives in
the diff.
The action facade's `find` accepted a caller-written `context` that
type-checked and the runtime did not honour — ADR-0049's
declared-but-unenforced shape on the one key that carries identity and
tenant. This takes the **remove** arm, at the declaration layer only:
the parameter becomes `Omit(EngineQueryOptions, 'context')`. **No
runtime behaviour changes.**
## The premise, measured FIRST — it HOLDS
The dispatch made the ruling conditional on a census: *no call site
writes a `context` on a facade query and relies on it to narrow identity
or tenant*. Measured before a line of fix was written.
**Instrument** (`census3.mjs`, three arms, run against the tree at
`1739f71879f`):
| Arm | What it matches | Why it exists |
|:---|:---|:---|
| A | `RECV.engine.VERB(` where `RECV` is an action-ctx name | the
canonical handler spelling |
| B | bare `engine.VERB(` in a file that destructures `engine` out of a
ctx | `examples/app-todo` writes it this way |
| C | `V.VERB(` where `V` is assigned from
`buildActionEngineFacade(...)` | closes arm A/B's blind spot: a facade
held in a local variable |
Each call's second argument is extracted by **balanced-paren scan**, not
a line regex, so a multi-line envelope is read whole.
**Radius**: 8292 tracked text files — the whole repository, not the
importers of `ActionEngineFacade`. That denominator is deliberate, and
it is the one PR #19223 warned about: the facade is reached through
`ActionHandlerContext.engine`, so an importer count of the facade type
is the wrong population.
**Readings**, exit codes captured before any pipe:
- facade call sites **123** (armA 86, armB 10, armC 27); of these **47
are `find`**
- sites writing a `context` key: **11**
- `find` sites writing a `context` key: **1**
That one is
`packages/runtime/src/action-engine-facade-find-envelope.test.ts:126` —
**the pin that asserts the key is NOT honoured**, added by #19223. It is
the instrument's **firing control**: arm C demonstrably sees a real
facade `find` carrying a `context`.
The other 10 are not facade sites, and each was classified by reading
the file rather than by name:
- 7 in `packages/objectql/src/internal-fields.test.ts` — `ctx` there is
`Awaited(ReturnType(typeof buildEngine))`, a **real ObjectQL engine**;
sibling calls to `findOne` and `aggregate` are members
`ActionEngineFacade` does not declare.
- 3 in `action-engine-facade-find-envelope.test.ts:209-211` — a
different `engine`, built by the file's own `makeRealEngine()`; they
pass a third argument, and the facade's `insert` takes two.
**Dark control**: the same instrument with a member and a builder that
cannot exist (`.engineZZZQ`, `buildActionEngineFacadeZZZQ`) — exit 1,
`FACADE_SITES total=0` on all three arms.
**Sibling radius**: `objectui` at `dda8f3815df` — `git grep` for
`ActionEngineFacade`, `ActionHandlerContext` and `ctx.engine.` exits **1
/ 0 hits**, with a firing control in the same tree (a token that
certainly exists) exiting 0.
⇒ **Zero live call sites.** The p0 upgrade trigger does not fire.
`priority:p1` stands.
## Mechanism: OVERRIDE, not drop — traced to a named line
At `origin/main` = `1739f71879f`, read 2026-09-20T08:42Z:
`packages/runtime/src/action-execution.ts:1620`
const rows = await ql.find(object, { ...(query ?? {}), context } as
any);
`context` is spread **last**, after the caller's envelope, so the
facade's own elevated `ExecutionContext` (minted at `:1560` by
`buildActionExecutionContext(ec)`) replaces whatever the caller put
under that key. The key reaches the engine; the caller's **value** does
not. PR #19223's body claim holds on today's tree, and it is override
rather than drop.
## The third card fact: the sibling arms do NOT share the shape
`ActionEngineFacade` declares exactly four members, and only one takes
an options bag:
insert(object, data) update(object, id, data)
delete(object, idOrIds) find(object, query) ← the only bag
There is no `findOne` and no `count` on this facade. The write doors
have nowhere to carry a `context` at the type level, so there is nothing
to price and nothing to widen this diff onto. Reported as measured, per
the order.
## What changed
- **`packages/spec/src/ui/action-params.zod.ts`** — the declaration.
`find(object, query: Omit(EngineQueryOptions, 'context'))`, plus the
member doc rewritten: why the key is gone, and the asymmetry it leaves.
- **`packages/spec/src/ui/action-params.test.ts`** — #15124's identity
pin retargeted to the narrowed shape; a second pin that reds **only**
when `context` becomes writable again; a value-level refusal pin with a
positive control.
- **`packages/runtime/src/action-execution.ts`** — **comment only, zero
behaviour.** The arm's docblock now states that the type no longer
admits the key while this arm still does, and why closing that half is
not a type narrowing's business.
- **`content/docs/ui/actions.mdx`** — the callout gains the one
subtraction.
- Generated: **none**. This PR originally regenerated
`api-surface-declarations/ui.txt`; main deleted that whole artefact
family (17 shards) in `2277d1fcd10`, so the regeneration was dropped in
the merge. The artefact that replaced it, `api-surface-signatures.json`,
does **not** move for this narrowing — `gen:api-surface` rewrites it
byte-identically (blob `b2099d11828`), because it hashes
`checker.typeToString()` of the 27 `defineX` factories, which prints a
type reference without expanding it.
## The pin is TYPE-level, and that is deliberate
`FindQueryCarriesNoContextKey` asserts the key is absent from the
declared slot; the two `@ts-expect-error` directives red if a literal
carrying `context` starts compiling. A **runtime** pin would assert a
refusal that does not exist and must not: adding one makes the facade
throw on an identity key, which is a runtime permission change no ruling
covers. The runtime's own pin is untouched and still green.
The file is inside the checked zone — `check:test-typecheck` reports
`packages/spec/tsconfig.test.json` compiling 54 files — so these are not
phantom directives.
## Reverse verification
Fix committed first, then the declaration alone reverted to
`EngineQueryOptions`:
- **on-disk proof** — narrowed spelling 1 → 0, widened 0 → 1, blob
`3f73de3ae0f` → `58f5dc6b90e`; a no-op edit would have been caught here
and the reading voided.
- **ablated** `pnpm --filter @objectstack/spec typecheck` → **exit 1**,
`src/ui/action-params.test.ts: 4 type error(s)` — the two asserts plus
the two now-unused `@ts-expect-error` directives.
- **restored** with `git checkout HEAD -- PATH` (never a bare checkout,
which reads the polluted index): `git diff HEAD` empty and `git
hash-object` back to `3f73de3ae0f`, byte-identical. A trap on
EXIT/INT/TERM carried the restore, with an absolute repo root.
Direction predicted before the run and observed: **red**.
## Verification — per consumer package, on the merged head `d55c3d9e772`
| Package | Reading |
|:---|:---|
| `@objectstack/spec` | 500 files / **14644** tests passed · `typecheck`
exit 0 |
| `@objectstack/runtime` | 268 files / **3705** passed, 1 skipped ·
`typecheck` exit 0 |
| `@objectstack/objectql` | 300 files / **5009** passed · `typecheck`
exit 0 |
| `@objectstack/example-todo` | 7 files / **238** passed · `typecheck`
exit 0 |
`@objectstack/objectql` is **not** on the dispatched floor list: the
census found it, at
`packages/objectql/src/engine-write-not-found-gate.test.ts`, which
builds a real facade through `buildActionEngineFacade`. Run because it
is a consumer, and said so.
⚠️ One reading was thrown away rather than reported: the first `runtime`
run answered *242 test files failed / 7 tests failed*, which was `Cannot
find package` on an unbuilt dependency closure — PREREQUISITE NOT MET,
not a red. Re-run after `pnpm --filter '@objectstack/example-todo^...'
build` and reported above.
**Gates.** `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack`, derived from this tree, re-derived after
the merge (same 107, no families added or dropped): **107 of 107
green**, each exit code redirected to its own file and read back before
any pipe, then reconciled with `--ran` carrying the codes — `107 run, 0
NOT-MEASURED (a DERIVED zero)`.
Two needed a second run, and both were prerequisite misses rather than
reds: `check:skill-examples` (exit 1, `packages/client-react/dist`
unbuilt) and `check:dual-build-cjs-loads` (exit 3, its own `PREREQUISITE
NOT MET — ⛔ This is NOT a pass`). Both green after building the missing
packages.
`check:pm-widening-tells` is **green** — the T1 tell that card #19099
records against this shape did not fire, so the `Clause-②: no`
declaration needed no over-declaring to get past a gate.
**Lint, repo-wide rather than narrowed:** `eslint . --no-inline-config`
over all **6916** files eslint's own config judges — **0 errors, 0
warnings**, exit 0, at `d55c3d9e772`. The file count is read from
eslint's own `--format json` output, not estimated. No type-aware
linting is configured (`eslint.config.mjs` states it carries no
`parserOptions.project` and no typed rules), so nothing in this diff can
move an untouched file's verdict.
## Declaration
`Clause-②: no` — this puts no new key on a published payload; it removes
one from a parameter type. The lane charter's line that a narrowing does
not trigger clause ② is the criterion, and `check:pm-widening-tells`
agrees with it mechanically. The **changeset** separately carries
`Clause-②: no (narrowing)`, which is signal (4) to
`check-adr-0087-registration`: an accept-set narrowing on a published
type is exactly what #16421 built that signal for, so it is declared
rather than left to prose, with an `already-registered` disposition
naming `action-engine-facade-find-query-envelope` — the entry #19223
landed, which already tells an upgrader that a caller-supplied `context`
is ignored. That gate is green.
## Acceptance notes
**Noted, not filed — the asymmetry this leaves, stated so nobody reads
it as an oversight.** After this diff the facade's `find` arm refuses
(at runtime) every top-level key the envelope does not carry,
accepts-and-honours the ones it does, and accepts-and-**overrides**
exactly one: `context`, for untyped callers only. Closing that last cell
means a runtime refusal on an identity key — the maintainer's floor, not
a dev's and not a seat's, and the dispatch prohibited taking it here. It
is recorded on both halves of the contract (the spec member doc and the
runtime arm's docblock, the latter with an explicit "do not finish the
job here without a ruling"). **Carrier: whoever holds the next ruling on
this surface** — there is no PR or person this file is waiting on today,
so it is written down where the next editor of either half will read it,
rather than filed as a card nobody is dispatched to.
⚠️ **Not a finding, but worth one line for the next census on this
surface:** a receiver-name heuristic over `.engine.` is not sound here —
10 of the 11 `context` writers it flags are the data engine, which
honours the key. Only a type or construction anchor
(`buildActionEngineFacade`, or the `findOne`/`aggregate` members the
facade lacks) separates the two populations.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01LvwGppdonww4zGLWZo5rho)_
---
_Generated by [Claude Code](https://claude.ai/code)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
`tsc --noEmit` over a consumer's handlers finds every occurrence, because the
33
+
key is now an excess property on a fresh literal. ⚠️ Only where the handler is
34
+
annotated with the published `ActionHandlerContext`: an untyped handler (a JS
35
+
config body, a local copy of the context type, `(ctx: any)`) still passes the
36
+
key and still has it overridden, silently, exactly as before. The runtime arm is
37
+
deliberately unchanged — refusing an identity key there is a runtime behaviour
38
+
change, not a declaration narrowing.
39
+
40
+
<!-- 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. -->
0 commit comments