Skip to content

Hook event taxonomy is over-specified: collapse 18 events → 8, make findOne fire find hooks, formalize bulk semantics #3195

Description

@os-zhuang

Summary (rewritten after design review)

Originally filed as "10 of 18 declared HookEvent values never dispatch." Investigation confirmed the gap, but the design review concluded the contract is wrong, not the runtime — the fix is to collapse the event taxonomy, not to implement the missing dispatch.

What investigation established

The dead 10 (beforeFindOne/afterFindOne, beforeCount/afterCount, beforeAggregate/afterAggregate, beforeUpdateMany/afterUpdateMany, beforeDeleteMany/afterDeleteMany): registered fine (engine.registerHook accepts any string, engine.ts:419; no registration-time dispatchability check anywhere), then silently never fire — only 8 literal triggerHooks() call sites exist (engine.ts:2094/2110/2231/2286/2403/2500/2697/2735).

Severity is lower than originally claimed — the platform has no security hole:

  • Platform security (RLS / permissions / aggregate gating) runs as middleware, which explicitly covers find/findOne/count/aggregate (security-plugin.ts:834, rls-compiler.ts:314, aggregate gate :1034). Hooks are not the security boundary.
  • External HTTP single-record reads go through ql.find(..., { where: { id }, limit: 1 }) (http-dispatcher.ts:378), notfindOne — so beforeFind/afterFind hooks DO fire for external reads. The findOne hook bypass affects only in-process callers (sandbox script API, automation nodes, internal services).
  • Bulk writes do not bypass hooks: beforeUpdate/beforeDelete/afterUpdate/afterDelete fire outside the single/multi branch (engine.ts:2403/2697). The *Many events name a code path that doesn't exist (the engine has no updateMany/deleteMany methods).
  • Zero real usage: no hook or authored metadata anywhere subscribes to any of the 10. But the skills teach 6 of them, including a copy-pasteable data-masking example subscribing to ['afterFind', 'afterFindOne'] (skills/objectstack-data/references/data-hooks.md:632) — the first user to follow it gets a silently no-op hook.

Design review: the taxonomy mirrors Mongoose's footgun, not Salesforce's lesson

Surveying how peers design data-layer interception:

  • Salesforce (our stated inspiration): triggers only on DML (before/after insert/update/delete/undelete), no read triggers (read side is declarative: sharing/FLS), and bulk-first — one event set, contexts are always collections; no single/bulk split.
  • PostgreSQL / Hasura: write triggers only; reads are RLS/permissions.
  • Rails: one after_find attached to record materialization — fires for every read shape, so one subscription covers all reads.
  • Django: write signals only; bulk explicitly documented as not firing.
  • Mongoose: per-query-method hooks (pre('find') vs pre('findOne') vs pre('updateMany') …) — a notorious FAQ generator ("why doesn't my find hook fire on findOne?"), mitigated upstream with regex subscriptions.

Our 18-value enum copies the Mongoose shape: it leaks the engine method table into the authoring contract instead of exposing domain events ("records are being read/written"). Consequence: any cross-cutting requirement must be authored N times (the skill example literally subscribes twice for one masking concern), and every extra event is an extra silent-mistake class for AI authors on a platform whose pitch is AI-safe metadata authoring.

The dead events' intended responsibilities all already have a correct home in our architecture:

Dead event's intended jobCorrect existing mechanism
beforeFindOne read authorizationmiddleware + RLS (covers findOne)
afterFindOne field maskingfield-level metadata (maskSecretFields is built into the read path)
beforeCount/beforeAggregate leakage guardssecurity middleware aggregate gate
*Many bulk interceptionsingular hooks already fire on bulk (context carries the predicate)

Decision

  1. Collapse the enum 18 → 8: keep before/after × insert/update/delete + beforeFind/afterFind. Trim the 10 dead members (same contract-first treatment as the validation events: ['delete'] trim, Validation events: ['delete'] is a silent no-op — trim it from the spec #3184; zero real usage ⇒ zero migration).
  2. Make findOne fire beforeFind/afterFind (Rails-style: the read event attaches to record materialization, not to the method name). One subscription then covers every read; the "write it twice" problem disappears. Purely fail-closed-direction behavior change; no existing hook is negatively affected.
  3. Formalize bulk semantics in the contract: document that the singular write events fire on multi: true operations with the row-scoping predicate in context (promote existing behavior to a promise, instead of inventing *Many twin events).
  4. Registration guard: warn when a hook subscribes to an event the engine never dispatches, so enum-vs-dispatch drift can't recur silently.
  5. Align skills/docs: teach the 8 events + a responsibility table (read filtering → RLS, masking → field metadata, delete guards → beforeDelete); fix the double-subscription masking example.

Prior-art gap

The 2026-06 HookSchema property-liveness audit checked property liveness, not enum-value liveness, and a docs-accuracy follow-up treated the dead values as real — this issue is the first treatment of the enum-vs-dispatch gap as a defect. Found during the PD #10 events-enum sweep after #3106/#3184; sibling findings: #3196 (webhook undelete/api), #3197 (schema-only event surfaces).

🤖 Generated with Claude Code

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingsecurity

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions