Skip to content

Commit a3823b2

Browse files
os-zhuangclaude
andauthored
fix(spec): collapse hook event taxonomy 18 → 8; findOne fires find hooks (#3206)
The HookEvent enum declared 18 events but the engine only ever dispatches 8. The 10 dead ones (beforeFindOne/afterFindOne, beforeCount/afterCount, beforeAggregate/afterAggregate, beforeUpdateMany/afterUpdateMany, beforeDeleteMany/afterDeleteMany) mirrored the engine method table rather than domain events, so a hook subscribing to them registered fine and silently never fired — the same declared≠enforced failure mode as the validation delete-event gap (#3184), and the skills even shipped a copy-pasteable afterFindOne masking example that no-ops. Design decision (see issue): collapse the taxonomy, don't build out dispatch. Peer platforms (Salesforce, Postgres, Rails) don't expose per-method read triggers or single/bulk splits — reads are one materialization event, writes are bulk-first. So: - findOne now fires the SAME beforeFind/afterFind hooks as find (the read event attaches to record materialization, not the method) — one subscription covers every read shape. - Bulk (multi:true) writes already fire the singular before/after write events with the row-scoping predicate in ctx.input.ast; documented, no *Many event. - Removed the 10 dead enum values. Read authz/row-filtering is the RLS/ permission layer; field masking is field metadata — not per-author hooks. - engine.registerHook warns when a hook subscribes to a non-dispatched event, so enum-vs-dispatch drift can't recur silently. No shipped hook or authored metadata used any removed event. Skills and docs (hooks.md, data-hooks.md, kernel/events.mdx, api/data-flow.mdx, protocol/objectql/schema.mdx) updated to teach 8 events + the declarative alternatives; regenerated content/docs/references/data/hook.mdx. Added tests for findOne firing find hooks and the registration guard. Closes #3195 Claude-Session: https://claude.ai/code/session_01VCUSMJBsX14C3RQdWFw7N7 Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8abf133 commit a3823b2

11 files changed

Lines changed: 207 additions & 82 deletions

File tree

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"@objectstack/spec": patch
3+
"@objectstack/objectql": patch
4+
---
5+
6+
Collapse the hook event taxonomy from 18 declared events to the 8 the engine actually dispatches (#3195). The removed 10 (`beforeFindOne`/`afterFindOne`, `beforeCount`/`afterCount`, `beforeAggregate`/`afterAggregate`, `beforeUpdateMany`/`afterUpdateMany`, `beforeDeleteMany`/`afterDeleteMany`) were declared in `HookEvent` but never fired — the enum mirrored the engine method table instead of domain events, so a hook subscribing to them registered fine and then silently no-op'd.
7+
8+
- `findOne` now fires the same `beforeFind`/`afterFind` hooks as `find` — the read event attaches to record materialization, not the engine method, so one subscription covers every read shape (no separate `beforeFindOne`/`afterFindOne`).
9+
- Bulk (`multi: true`) updates/deletes already fire the singular `beforeUpdate`/`beforeDelete`/`afterUpdate`/`afterDelete` events with the row-scoping predicate in `ctx.input.ast`; this is now documented, and there is no `*Many` event.
10+
- Read authorization / row filtering is the RLS/permission-rule layer's job and field masking is field-level metadata — neither is a hook every author must re-attach.
11+
- `engine.registerHook` now warns when a hook subscribes to an event the engine never dispatches, so enum-vs-dispatch drift can't recur silently.
12+
13+
No shipped hook or authored metadata used any of the removed events; authoring one now fails loudly at parse/validate time instead of registering a dead hook. Skills and docs updated to teach the 8 events and the declarative alternatives.

content/docs/api/data-flow.mdx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ sequenceDiagram
9696
9797
QE-->>K: DataRecord
9898
99-
K->>H: afterFindOne hooks
99+
K->>H: afterFind hooks (fire for findOne too)
100100
H-->>K: Transformed record
101101
102102
K->>Sec: applyFieldSecurity(record, user)
@@ -310,7 +310,7 @@ flowchart TD
310310
end
311311
```
312312

313-
The lifecycle events are defined by the `HookEvent` enum. Reads also have hooks: `beforeFind/afterFind`, `beforeFindOne/afterFindOne`, `beforeCount/afterCount`, and `beforeAggregate/afterAggregate`. Bulk writes add `beforeUpdateMany/afterUpdateMany` and `beforeDeleteMany/afterDeleteMany`.
313+
The lifecycle events are defined by the `HookEvent` enum (8 events). Reads fire `beforeFind`/`afterFind` — for **both** `find` and `findOne`, so one subscription covers every read shape. Bulk writes (`multi: true`) fire the **same** `beforeUpdate`/`beforeDelete`/`afterUpdate`/`afterDelete` events as single-id writes, with the row-scoping predicate in `ctx.input.ast`. There are deliberately no per-method (`findOne`/`count`/`aggregate`) or `*Many` events: read authorization and row filtering are RLS/permission-rule concerns, and field masking is field-level metadata.
314314

315315
| Hook | Phase | Can Modify? | Can Abort? |
316316
|:---|:---|:---|:---|

content/docs/kernel/events.mdx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,10 @@ The Data Engine runs hooks around record reads and mutations. Write operations f
5858
| Event | When |
5959
| :--- | :--- |
6060
| `beforeInsert` / `afterInsert` | Around record creation. |
61-
| `beforeUpdate` / `afterUpdate` | Around record update. |
62-
| `beforeDelete` / `afterDelete` | Around record deletion. |
61+
| `beforeUpdate` / `afterUpdate` | Around record update — single-id **and** bulk (`multi: true`). |
62+
| `beforeDelete` / `afterDelete` | Around record deletion — single-id **and** bulk (`multi: true`). |
6363

64-
Read and bulk variants (`beforeFind`/`afterFind`, `beforeCount`, `beforeAggregate`, `beforeUpdateMany`, `beforeDeleteMany`, …) also exist.
64+
Reads fire `beforeFind` / `afterFind`, for **both** `find` and `findOne` (one read event covers every read shape). There are no per-method (`findOne`/`count`/`aggregate`) or `*Many` events: read authorization and row filtering are handled by RLS/permission rules, field masking by field-level metadata, and bulk writes by the same `before*`/`after*` write events (with the row-scoping predicate in `ctx.input.ast`).
6565

6666
### The HookContext
6767

content/docs/protocol/objectql/schema.mdx

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -594,12 +594,15 @@ const customerHook: Hook = {
594594
export default customerHook;
595595
```
596596

597-
**Event Types** (camelCase):
597+
**Event Types** (camelCase) — 8 events:
598598
- `beforeInsert` / `afterInsert`
599-
- `beforeUpdate` / `afterUpdate`
600-
- `beforeDelete` / `afterDelete`
601-
- Query-side: `beforeFind` / `afterFind`, `beforeCount` / `afterCount`, and the
602-
bulk variants `beforeUpdateMany` / `beforeDeleteMany`, etc.
599+
- `beforeUpdate` / `afterUpdate` — fire for single-id **and** bulk (`multi: true`) updates
600+
- `beforeDelete` / `afterDelete` — fire for single-id **and** bulk (`multi: true`) deletes
601+
- `beforeFind` / `afterFind` — fire for **both** `find` and `findOne`
602+
603+
There are no per-method (`findOne` / `count` / `aggregate`) or `*Many` events: read
604+
authorization and row filtering are RLS/permission-rule concerns, field masking is
605+
field-level metadata, and bulk writes reuse the singular write events.
603606

604607
## Advanced Features
605608

content/docs/references/data/hook.mdx

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ const result = HookContext.parse(data);
3333
| :--- | :--- | :--- | :--- |
3434
| **id** | `string` | optional | Unique execution ID for tracing |
3535
| **object** | `string` || |
36-
| **event** | `Enum<'beforeFind' \| 'afterFind' \| 'beforeFindOne' \| 'afterFindOne' \| 'beforeCount' \| 'afterCount' \| 'beforeAggregate' \| 'afterAggregate' \| 'beforeInsert' \| 'afterInsert' \| 'beforeUpdate' \| 'afterUpdate' \| 'beforeDelete' \| 'afterDelete' \| 'beforeUpdateMany' \| 'afterUpdateMany' \| 'beforeDeleteMany' \| 'afterDeleteMany'>` || |
36+
| **event** | `Enum<'beforeFind' \| 'afterFind' \| 'beforeInsert' \| 'afterInsert' \| 'beforeUpdate' \| 'afterUpdate' \| 'beforeDelete' \| 'afterDelete'>` || |
3737
| **input** | `Record<string, any>` || Mutable input parameters |
3838
| **result** | `any` | optional | Operation result (After hooks only) |
3939
| **previous** | `Record<string, any>` | optional | Record state before operation |
@@ -52,22 +52,12 @@ const result = HookContext.parse(data);
5252

5353
* `beforeFind`
5454
* `afterFind`
55-
* `beforeFindOne`
56-
* `afterFindOne`
57-
* `beforeCount`
58-
* `afterCount`
59-
* `beforeAggregate`
60-
* `afterAggregate`
6155
* `beforeInsert`
6256
* `afterInsert`
6357
* `beforeUpdate`
6458
* `afterUpdate`
6559
* `beforeDelete`
6660
* `afterDelete`
67-
* `beforeUpdateMany`
68-
* `afterUpdateMany`
69-
* `beforeDeleteMany`
70-
* `afterDeleteMany`
7161

7262

7363
---

packages/objectql/src/engine.test.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,52 @@ describe('ObjectQL Engine', () => {
530530
});
531531
});
532532

533+
describe('Read hooks on findOne + registration guard (#3195)', () => {
534+
beforeEach(async () => {
535+
engine.registerDriver(mockDriver, true);
536+
await engine.init();
537+
vi.mocked(SchemaRegistry.getObject).mockReturnValue({ name: 'task', fields: {} } as any);
538+
});
539+
540+
it('findOne fires beforeFind and afterFind (one read event covers find and findOne)', async () => {
541+
(mockDriver.findOne as any).mockResolvedValue({ id: 't1', name: 'raw' });
542+
const events: string[] = [];
543+
engine.registerHook('beforeFind', async () => { events.push('beforeFind'); }, { object: 'task' });
544+
engine.registerHook('afterFind', async () => { events.push('afterFind'); }, { object: 'task' });
545+
546+
await engine.findOne('task', { where: { id: 't1' } } as any);
547+
548+
expect(events).toEqual(['beforeFind', 'afterFind']);
549+
});
550+
551+
it('afterFind hook can transform the findOne result', async () => {
552+
(mockDriver.findOne as any).mockResolvedValue({ id: 't1', name: 'raw' });
553+
engine.registerHook('afterFind', async (ctx: any) => {
554+
if (ctx.result) ctx.result.name = 'masked';
555+
}, { object: 'task' });
556+
557+
const out = await engine.findOne('task', { where: { id: 't1' } } as any);
558+
559+
expect(out).toMatchObject({ id: 't1', name: 'masked' });
560+
});
561+
562+
it('warns when a hook subscribes to an event the engine never dispatches', () => {
563+
const warn = vi.spyOn((engine as any).logger, 'warn');
564+
engine.registerHook('beforeFindOne', async () => {}, { object: 'task' });
565+
expect(warn).toHaveBeenCalledWith(
566+
expect.stringContaining("'beforeFindOne'"),
567+
expect.objectContaining({ event: 'beforeFindOne' }),
568+
);
569+
});
570+
571+
it('does not warn for a dispatchable event', () => {
572+
const warn = vi.spyOn((engine as any).logger, 'warn');
573+
engine.registerHook('beforeUpdate', async () => {}, { object: 'task' });
574+
const warnedForUpdate = warn.mock.calls.some((c) => String(c[0]).includes("'beforeUpdate'"));
575+
expect(warnedForUpdate).toBe(false);
576+
});
577+
});
578+
533579
describe('Update hooks — previous-record snapshot', () => {
534580
beforeEach(async () => {
535581
engine.registerDriver(mockDriver, true);

packages/objectql/src/engine.ts

Lines changed: 51 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,22 @@ import { validateRecord, normalizeMultiValueFields, coerceBooleanFields, Validat
4646
import { evaluateValidationRules, needsPriorRecord, stripReadonlyWhenFields, stripReadonlyWhenFieldsMulti, hasReadonlyWhenInPayload, stripReadonlyFields } from './validation/rule-validator.js';
4747
import { applyInMemoryAggregation } from './in-memory-aggregation.js';
4848

49+
/**
50+
* The lifecycle events the engine actually dispatches via `triggerHooks`. This
51+
* is the single source of truth for what a hook can subscribe to — kept in
52+
* lockstep with the `triggerHooks(...)` call sites and with `HookEvent` in the
53+
* spec. `beforeFind`/`afterFind` cover both `find` and `findOne`; the write
54+
* events cover both single-id and bulk (`multi: true`) writes (#3195). A hook
55+
* subscribing to anything outside this set would silently never fire, so
56+
* `registerHook` warns rather than accepting it blindly.
57+
*/
58+
const DISPATCHABLE_HOOK_EVENTS: ReadonlySet<string> = new Set([
59+
'beforeFind', 'afterFind',
60+
'beforeInsert', 'afterInsert',
61+
'beforeUpdate', 'afterUpdate',
62+
'beforeDelete', 'afterDelete',
63+
]);
64+
4965
interface FormulaPlanEntry { name: string; expression: Expression; }
5066

5167
function planFormulaProjection(
@@ -437,6 +453,18 @@ export class ObjectQL implements IDataEngine {
437453
/** Stable name from metadata (set by `bindHooksToEngine`). */
438454
hookName?: string;
439455
}) {
456+
// [#3195] Guard against enum-vs-dispatch drift: a hook on an event the
457+
// engine never triggers would register "successfully" and then silently
458+
// never fire. Warn loudly rather than swallow it. Not a hard reject — a
459+
// custom driver/plugin may dispatch its own events via `triggerHooks`.
460+
if (!DISPATCHABLE_HOOK_EVENTS.has(event)) {
461+
this.logger.warn(
462+
`Hook registered for '${event}', which the engine never dispatches — it will never fire. ` +
463+
`Dispatchable events: ${[...DISPATCHABLE_HOOK_EVENTS].join(', ')}. ` +
464+
`(Read filtering → RLS/permissions; field masking → field metadata; delete guards → beforeDelete.)`,
465+
{ event, object: options?.object, hookName: options?.hookName },
466+
);
467+
}
440468
if (!this.hooks.has(event)) {
441469
this.hooks.set(event, []);
442470
}
@@ -2207,8 +2235,23 @@ export class ObjectQL implements IDataEngine {
22072235
};
22082236

22092237
await this.executeWithMiddleware(opCtx, async () => {
2210-
const findOneOpts = this.buildDriverOptions(opCtx.context);
2211-
let result = await driver.findOne(objectName, opCtx.ast as QueryAST, findOneOpts);
2238+
// [#3195] `findOne` fires the SAME `beforeFind`/`afterFind` hooks as
2239+
// `find` — the read event attaches to record materialization, not to the
2240+
// engine method, so one subscription covers every read shape (there is no
2241+
// separate `beforeFindOne`/`afterFindOne`). Mirrors `find()` above.
2242+
const hookContext: HookContext = {
2243+
object: objectName,
2244+
event: 'beforeFind',
2245+
input: { ast: opCtx.ast, options: opCtx.options },
2246+
session: this.buildSession(opCtx.context),
2247+
api: this.buildHookApi(opCtx.context),
2248+
transaction: opCtx.context?.transaction,
2249+
ql: this
2250+
};
2251+
await this.triggerHooks('beforeFind', hookContext);
2252+
hookContext.input.options = this.buildDriverOptions(opCtx.context, hookContext.input.options as any);
2253+
2254+
let result = await driver.findOne(objectName, hookContext.input.ast as QueryAST, hookContext.input.options as any);
22122255

22132256
// Post-process: evaluate formula virtual fields against the raw row
22142257
if (result != null) applyFormulaPlan(_findOneFormula.plan, [result], opCtx.context);
@@ -2219,10 +2262,14 @@ export class ObjectQL implements IDataEngine {
22192262
result = expanded[0];
22202263
}
22212264

2265+
hookContext.event = 'afterFind';
2266+
hookContext.result = result;
2267+
await this.triggerHooks('afterFind', hookContext);
2268+
22222269
// Mask secret fields — plaintext never leaves through the read path.
2223-
this.maskSecretFields(objectName, result);
2270+
this.maskSecretFields(objectName, hookContext.result);
22242271

2225-
return result;
2272+
return hookContext.result;
22262273
});
22272274

22282275
return opCtx.result;

packages/spec/src/data/hook.test.ts

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,8 @@ import {
99

1010
describe('HookEvent', () => {
1111
describe('Read Operations', () => {
12-
it('should accept read operation events', () => {
13-
const readEvents = [
14-
'beforeFind', 'afterFind',
15-
'beforeFindOne', 'afterFindOne',
16-
'beforeCount', 'afterCount',
17-
'beforeAggregate', 'afterAggregate',
18-
];
12+
it('should accept read events (fire for both find and findOne)', () => {
13+
const readEvents = ['beforeFind', 'afterFind'];
1914

2015
readEvents.forEach(event => {
2116
expect(() => HookEvent.parse(event)).not.toThrow();
@@ -24,7 +19,7 @@ describe('HookEvent', () => {
2419
});
2520

2621
describe('Write Operations', () => {
27-
it('should accept write operation events', () => {
22+
it('should accept write events (fire for both single and bulk writes)', () => {
2823
const writeEvents = [
2924
'beforeInsert', 'afterInsert',
3025
'beforeUpdate', 'afterUpdate',
@@ -37,15 +32,22 @@ describe('HookEvent', () => {
3732
});
3833
});
3934

40-
describe('Bulk Operations', () => {
41-
it('should accept bulk operation events', () => {
42-
const bulkEvents = [
35+
describe('Removed non-dispatched events (#3195)', () => {
36+
it('should reject per-method read and *Many events that the engine never dispatched', () => {
37+
// These were declared but never fired; the engine only ever triggers the
38+
// 8 events above. Removed rather than left as silent no-ops — read
39+
// filtering is RLS/middleware, masking is field metadata, and bulk writes
40+
// fire the singular before/after events.
41+
const removed = [
42+
'beforeFindOne', 'afterFindOne',
43+
'beforeCount', 'afterCount',
44+
'beforeAggregate', 'afterAggregate',
4345
'beforeUpdateMany', 'afterUpdateMany',
4446
'beforeDeleteMany', 'afterDeleteMany',
4547
];
4648

47-
bulkEvents.forEach(event => {
48-
expect(() => HookEvent.parse(event)).not.toThrow();
49+
removed.forEach(event => {
50+
expect(() => HookEvent.parse(event)).toThrow();
4951
});
5052
});
5153
});

packages/spec/src/data/hook.zod.ts

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -10,20 +10,25 @@ import { ExpressionInputSchema } from '../shared/expression.zod';
1010
import { lazySchema } from '../shared/lazy-schema';
1111
import { HookBodySchema } from './hook-body.zod';
1212
export const HookEvent = z.enum([
13-
// Read Operations
13+
// Read — one event per read, regardless of shape. `beforeFind`/`afterFind`
14+
// fire for BOTH `find` and `findOne` (the event attaches to record
15+
// materialization, not to the engine method — Rails' `after_find` model), so
16+
// a single subscription covers every read path. There is deliberately no
17+
// per-method read event (`findOne`/`count`/`aggregate`): read authorization
18+
// and row filtering are the RLS/permission middleware's job, and field
19+
// masking is field-level metadata — not something every author re-implements
20+
// as a hook. See #3195.
1421
'beforeFind', 'afterFind',
15-
'beforeFindOne', 'afterFindOne',
16-
'beforeCount', 'afterCount',
17-
'beforeAggregate', 'afterAggregate',
1822

19-
// Write Operations
23+
// Write — before/after per mutation kind. These fire on BOTH single-id and
24+
// bulk (`multi: true`) writes: a bulk update/delete runs the SAME
25+
// `beforeUpdate`/`beforeDelete`/`afterUpdate`/`afterDelete` with the
26+
// row-scoping predicate carried in `input` (there is no per-cardinality
27+
// `*Many` event — one write event covers one row or many, Salesforce's
28+
// bulk-first model). See #3195.
2029
'beforeInsert', 'afterInsert',
2130
'beforeUpdate', 'afterUpdate',
2231
'beforeDelete', 'afterDelete',
23-
24-
// Bulk Operations (Query-based)
25-
'beforeUpdateMany', 'afterUpdateMany',
26-
'beforeDeleteMany', 'afterDeleteMany',
2732
]);
2833

2934
/**
@@ -171,16 +176,21 @@ export const HookContextSchema = lazySchema(() => z.object({
171176
/** Current Lifecycle Event */
172177
event: HookEvent,
173178

174-
/**
179+
/**
175180
* Input Parameters (Mutable)
176181
* Modify this to change the behavior of the operation.
177-
*
178-
* - find: { query: QueryAST, options: DriverOptions }
182+
*
183+
* - find (also fires for findOne): { ast: QueryAST, options: DriverOptions }
179184
* - insert: { doc: Record, options: DriverOptions }
180-
* - update: { id: ID, doc: Record, options: DriverOptions }
181-
* - delete: { id: ID, options: DriverOptions }
182-
* - updateMany: { query: QueryAST, doc: Record, options: DriverOptions }
183-
* - deleteMany: { query: QueryAST, options: DriverOptions }
185+
* - update (single id): { id: ID, data: Record, options: DriverOptions }
186+
* - update (bulk, multi:true): { ast: QueryAST, data: Record, options: DriverOptions }
187+
* - delete (single id): { id: ID, options: DriverOptions }
188+
* - delete (bulk, multi:true): { ast: QueryAST, options: DriverOptions }
189+
*
190+
* A bulk (`multi: true`) update/delete fires the SAME `beforeUpdate`/
191+
* `beforeDelete` events as a single-id write — the row-scoping predicate is
192+
* carried in `input.ast` (no per-row `id`). There is no separate `*Many`
193+
* event.
184194
*/
185195
input: z.record(z.string(), z.unknown()).describe('Mutable input parameters'),
186196

0 commit comments

Comments
 (0)