Skip to content

Commit f794e4e

Browse files
claude[bot]claude
andauthored
fix(spec): type ActionEngineFacade.find's second parameter as a FilterCondition, not an ObjectQL envelope (#14175) (#15118)
* wip: type ActionEngineFacade.find's filter parameter (#14175) * wip: hoist the type-level pin to an exported alias (#14175) --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent f594e70 commit f794e4e

4 files changed

Lines changed: 164 additions & 1 deletion

File tree

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
---
2+
"@objectstack/spec": patch
3+
---
4+
5+
fix(spec): `ActionEngineFacade.find` declares its second parameter as a FILTER, not an ObjectQL envelope (#14175)
6+
7+
`find(object, query: Record<string, unknown>)` documented nothing, and its
8+
parameter carried the name of the envelope every other read on the platform
9+
takes. The runtime (`buildActionEngineFacade`,
10+
`packages/runtime/src/action-execution.ts`) treats the argument as the bare
11+
`where` half — wrapping a non-empty one as `{ where: filter }` and passing
12+
`{}` through unwrapped — so a handler that passed the envelope got
13+
`{ where: { where: … } }`, matched nothing and returned `[]` with no error,
14+
while its one unfiltered read kept working. A hand-written test double built
15+
on the same belief passed every assertion; an application's headline action
16+
was a silent no-op for its whole life under a green suite.
17+
18+
The member is now `find(object, filter: FilterCondition)` — the published
19+
`QueryAST.where` type — with a doc comment stating the contract, the runtime's
20+
wrap, and both limbs (envelope wrapped; empty passed through); the facade
21+
docblock points at it. The parameter's TYPE now says what the runtime does
22+
at the one place a handler author reads.
23+
24+
Compile-layer signal only, shipped as `patch` (the #12615 precedent — a
25+
compile-time narrowing with no change in what parses or runs): no runtime
26+
behaviour changes, nothing changes in what the facade accepts or returns, and
27+
the narrowing bites only a primitive or a mistyped `$and` / `$or` / `$not`.
28+
⚠️ It does NOT refuse `{ where: … }` at compile time — `FilterCondition`'s
29+
string index signature admits `where` as a field name — so the compile-time
30+
bar is partial and the doc comment is the contract of record. An
31+
implementation typed with the old `Record<string, unknown>` still satisfies
32+
the interface (method parameters are bivariant), so nothing constructing the
33+
facade changes.

content/docs/ui/actions.mdx

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,21 @@ export async function completeTask(ctx: ActionContext): Promise<void> {
146146
}
147147
```
148148

149+
<Callout type="warn">
150+
**`ctx.engine.find(object, filter)` takes a filter, not a query.** The second
151+
argument is the `where` half only — `{ status: 'completed' }`, operators
152+
(`{ amount: { $gt: 100 } }`), `$and` / `$or` / `$not` — and the runtime wraps
153+
it in `where` itself. Passing an ObjectQL envelope
154+
(`{ where: { status: 'completed' } }`) raises no error: it becomes
155+
`{ where: { where: … } }`, matches no row, and returns `[]`. An empty filter
156+
(`{}`) is passed through unwrapped, so the one unfiltered read works under
157+
either reading and a handler can look partially alive. The parameter is typed
158+
`FilterCondition` (`ActionEngineFacade` in `@objectstack/spec/ui`), which
159+
refuses a primitive or a mistyped `$and` / `$or` / `$not` but still admits
160+
`where` as a key — the sentence above is the contract, and a hand-written test
161+
double must honour it too.
162+
</Callout>
163+
149164
```typescript title="objectstack.config.ts"
150165
export const onEnable = async (ctx: { ql: { registerAction: (...args: unknown[]) => void } }) => {
151166
ctx.ql.registerAction('todo_task', 'completeTask', completeTask);

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

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,11 @@ import {
55
validateActionParams,
66
ACTION_PARAM_BUILTIN_KEYS,
77
ActionSessionSchema,
8+
type ActionEngineFacade,
89
type ActionSession,
910
type ResolvedActionParam,
1011
} from './action-params.zod';
12+
import type { FilterCondition } from '../data/filter.zod';
1113
import { MIGRATIONS_BY_MAJOR } from '../migrations/registry';
1214

1315
const codes = (issues: ReturnType<typeof validateActionParams>) => issues.map((i) => i.code).sort();
@@ -392,3 +394,77 @@ describe('#5779 — ActionSession `positions` canonical + `roles` deprecated ali
392394
expect(entry!.acceptanceCriteria).toMatch(/ctx\.session\.positions/);
393395
});
394396
});
397+
398+
// ---------------------------------------------------------------------------
399+
// #14175 — `ActionEngineFacade.find` takes a FILTER, never an ObjectQL envelope
400+
// ---------------------------------------------------------------------------
401+
402+
type Eq< A, B > = (< T >() => T extends A ? 1 : 2) extends (< T >() => T extends B ? 1 : 2) ? true : false;
403+
type Assert< T extends true > = T;
404+
405+
// The declared slot, read off the interface — not a retyped copy of it, so a
406+
// re-widening back to an open record, or a rename of the type behind it, fails
407+
// HERE rather than in the first consumer to notice.
408+
type FindFilter = Parameters<ActionEngineFacade['find']>[1];
409+
410+
// The type-level pin (the tsc channel, `tsc -p tsconfig.test.json`). `Eq` is
411+
// the strict mutual-assignability test, so `Record<string, unknown>` — the type
412+
// this slot carried before, and the one it must not drift back to — does not
413+
// satisfy it (measured: the same `Assert` against `Record<string, unknown>` is
414+
// a TS2344). Exported, as the sibling pins are, so `noUnusedLocals` does not
415+
// read a type that exists only to be checked as one that is never used.
416+
export type FindFilterIsFilterCondition = Assert< Eq< FindFilter, FilterCondition > >;
417+
418+
describe('#14175 — ActionEngineFacade.find takes a FILTER (the `where` half), never an ObjectQL envelope', () => {
419+
it('types the second parameter as the published `FilterCondition` (the tsc channel)', () => {
420+
// The value-level half of `FindFilterIsFilterCondition` above: a literal
421+
// annotated with the slot type, so the runtime run exercises the same
422+
// declaration the type pin reads.
423+
const filter: FindFilter = { position_code: 'qa_lead', active: true };
424+
expect(Object.keys(filter)).toEqual(['position_code', 'active']);
425+
});
426+
427+
it('positive control — every shape a handler legitimately passes compiles, the empty filter included', () => {
428+
const implicitEquality: FindFilter = { status: 'completed' };
429+
const explicitOperator: FindFilter = { position_code: { $in: ['qa_lead', 'qa_manager'] }, active: true };
430+
const logical: FindFilter = {
431+
$and: [{ active: true }, { $or: [{ tier: 'a' }, { tier: 'b' }] }],
432+
$not: { archived: true },
433+
};
434+
// The runtime passes THIS one through unwrapped — the unfiltered read, and
435+
// the one call that kept working in the reporting app under either belief.
436+
const unfiltered: FindFilter = {};
437+
438+
expect([implicitEquality, explicitOperator, logical, unfiltered].every((f) => typeof f === 'object')).toBe(true);
439+
});
440+
441+
it('refuses at compile time what `FilterCondition` refuses — a primitive and a mistyped logical operator', () => {
442+
// Each `@ts-expect-error` is itself checked: if the type ever ADMITS one of
443+
// these, the directive goes unused and `tsc -p tsconfig.test.json` reds.
444+
// @ts-expect-error — a filter is an object; a bare string is not a `where` half.
445+
const primitive: FindFilter = 'position_code = qa_lead';
446+
// @ts-expect-error — `$and` is `FilterCondition[]`; a string is refused.
447+
const andNotArray: FindFilter = { $and: 'active' };
448+
// @ts-expect-error — `$or` is `FilterCondition[]`; a bare object is refused.
449+
const orNotArray: FindFilter = { $or: { active: true } };
450+
// @ts-expect-error — `$not` is a `FilterCondition`; a string is refused.
451+
const notNotFilter: FindFilter = { $not: 'archived' };
452+
453+
expect([primitive, andNotArray, orNotArray, notNotFilter]).toHaveLength(4);
454+
});
455+
456+
it('MEASURED GAP — the exact envelope mistake still compiles; the doc comment, not the type, is the contract', () => {
457+
// `FilterCondition`'s string index signature is what lets a field NAME be a
458+
// key, and `where` is a string — so the shape that returned `[]` in silence
459+
// in the reporting app (`{ where: { position_code: 'qa_lead' } }`) is
460+
// admitted by the type, one level down too. This pin RECORDS that
461+
// measurement rather than hiding it: a later narrowing that refuses `where`
462+
// at the top level turns it red on purpose, so the member's "does NOT
463+
// refuse `{ where: … }`" sentence is updated with the type instead of
464+
// drifting from it.
465+
const envelope: FindFilter = { where: { position_code: 'qa_lead' } };
466+
const nested: FindFilter = { where: { where: { position_code: 'qa_lead' } } };
467+
468+
expect('where' in envelope && 'where' in nested).toBe(true);
469+
});
470+
});

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

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import { z } from 'zod';
3030

3131
import { valueSchemaFor } from '../data/field-value.zod';
32+
import type { FilterCondition } from '../data/filter.zod';
3233
import type { FieldErrorCode } from '../api/errors.zod';
3334
import { lazySchema } from '../shared/lazy-schema';
3435

@@ -229,12 +230,50 @@ export function validateActionParams(
229230
* The slim engine facade an action handler's `ctx.engine` exposes. TRUSTED —
230231
* context-less, RLS/FLS-bypassing by design (#2849); the boundary is enforced
231232
* at invoke time (`ai.exposed` + the ADR-0066 D4 capability gate), not here.
233+
*
234+
* `find` is the one member whose argument shape the signature alone never
235+
* settled: it takes a bare FILTER — the `where` half of a query — and never
236+
* an ObjectQL query envelope; read its doc comment before writing a handler
237+
* or a test double against it (#14175).
232238
*/
233239
export interface ActionEngineFacade {
234240
insert(object: string, data: Record<string, unknown>): Promise<{ id: string }>;
235241
update(object: string, id: string, data: Record<string, unknown>): Promise<void>;
236242
delete(object: string, id: string): Promise<void>;
237-
find(object: string, query: Record<string, unknown>): Promise<Array<Record<string, unknown>>>;
243+
/**
244+
* Read the rows of `object` that match `filter`.
245+
*
246+
* `filter` is a FILTER — the `where` HALF of an ObjectQL query, the same
247+
* {@link FilterCondition} that `QueryAST.where` carries: implicit equality
248+
* `{ field: value }`, explicit operators `{ field: { $in: [...] } }`,
249+
* `$and` / `$or` / `$not`. It is NOT the query ENVELOPE
250+
* (`{ where, fields, orderBy, limit }`) that `DataEngine.find` and ObjectQL's
251+
* own `engine.find` take — the shape this parameter's former name, `query`,
252+
* invited. The runtime builds the envelope itself: `buildActionEngineFacade`'s
253+
* `find` arm (`packages/runtime/src/action-execution.ts`, `:1183` on
254+
* `369da918`) wraps a non-empty filter as `{ where: filter }` and passes an
255+
* EMPTY filter (`{}`) through unwrapped — the unfiltered read.
256+
*
257+
* Two consequences, both silent (#14175):
258+
*
259+
* - An envelope passed here becomes `{ where: { where: … } }`. No object has
260+
* a field named `where`, so the read matches nothing and resolves to `[]`
261+
* with no error. A handler that made this mistake ran to completion over
262+
* zero rows for as long as it shipped, and its own hand-written test
263+
* double — written to the same belief, reading `query.where` — passed
264+
* every assertion.
265+
* - Because `{}` skips the wrap, an unfiltered call works under EITHER
266+
* reading, so a handler mixing one unfiltered read with envelope-shaped
267+
* ones looks partially alive rather than uniformly dead.
268+
*
269+
* What the type buys, exactly: `FilterCondition` refuses a primitive and a
270+
* mistyped logical operator (`$and` / `$or` not arrays, `$not` not a
271+
* filter). It does NOT refuse `{ where: … }` — its string index signature is
272+
* what lets any field name stand as a key, and `where` is a string — so the
273+
* envelope mistake still compiles, and this doc comment, not the type, is
274+
* the contract of record. Both halves are pinned in `action-params.test.ts`.
275+
*/
276+
find(object: string, filter: FilterCondition): Promise<Array<Record<string, unknown>>>;
238277
}
239278

240279
/**

0 commit comments

Comments
 (0)