diff --git a/.changeset/hook-exclude-objects-registration-face.md b/.changeset/hook-exclude-objects-registration-face.md new file mode 100644 index 0000000000..2235f1cd21 --- /dev/null +++ b/.changeset/hook-exclude-objects-registration-face.md @@ -0,0 +1,16 @@ +--- +'@objectstack/objectql': minor +'@objectstack/spec': minor +--- + +A hook registration can now express "global, EXCEPT these objects" — `registerHook(event, handler, { excludeObjects })`. + +`registerHook` carried one scope face: `object`, an allow list (absent = global, `'*'` = every object). An allow list and a deny list are interchangeable only over a closed universe of object names, and this one is open — a successful `/meta` PUT registers new objects into a running engine, and `SchemaRegistry.registerObject` emits no event a plugin could subscribe to. So a registrant wanting "everything except these platform tables" had two options, both wrong: keep the skip list inside the handler as an early return, which leaves the registration global and makes the per-object gates (`hasHooksFor`, the bulk-write row-set read) answer "hooks apply" for objects the handler is about to skip; or enumerate the complement into `object`, which freezes the list at boot so an object created afterwards is silently not covered — a compliance regression for the audit plugin, and a silent one. + +`excludeObjects?: string | string[]` is the deny half, subtracted from whatever `object` admits: `matches = allowMatches && !excludeMatches`. Absent means subtract nothing, so every registration that compiled before still behaves identically. Declared on the registration rather than left to a predicate callback, so the scope stays static, printable — the `Registered hook` debug record now reports it — and introspectable by diagnostics. + +Two shapes are refused at registration, following the same reasoning as the empty-target ruling: an empty name (`''`, `['']`, or a blank member) would subtract nothing while reading as though it subtracted something, and `'*'` would subtract every object and leave a hook that can never fire (ADR-0078: no silently inert declaration). Both throw, naming the fix. `excludeObjects: []` is accepted — it is the honest spelling of "subtract nothing", and the natural value of a spread whose source list is empty. + +`triggerHooks` (dispatch) and `hasHooksFor` (the bulk-write gate) were two hand-written copies of one matching semantic; adding a second scope dimension to two copies is how they drift, so both now call one shared matcher. A property test pins the direction that matters — the gate is never tighter than the dispatch, since a looser gate costs a wasted query while a tighter one silently drops hooks that were going to fire. + +The authorable `HookSchema` is deliberately untouched: the consumer is plugin code registering in TypeScript, and no metadata author needs "global minus a list" today. The key stays off the authoring surface until real pull appears. diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index f40767a031..b9abb482df 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -698,6 +698,22 @@ export type HookHandler = (context: HookContext) => Promise | void; export interface HookEntry { handler: HookHandler; object?: string | string[]; // undefined = global hook + /** + * [#5928] Object name(s) SUBTRACTED from whatever `object` admits — the + * negative half of a registration's scope. Absent = subtract nothing. + * + * `object` alone can only say "these objects" (plus the `'*'` full set), and + * the two are interchangeable only over a CLOSED universe. This one is open: + * `applyObjectRegistryMutation` registers objects into a running engine on a + * successful `/meta` PUT, emitting no event a plugin could subscribe to, so a + * registrant that enumerated the complement of its skip list would keep a + * frozen list and silently stop covering every object registered after boot. + * For the audit plugin that is a compliance regression, and a silent one. + * + * Read only through {@link hookMatchesObject} — never re-implemented at a + * call site (see that function's note on the one-semantic rule). + */ + excludeObjects?: string | string[]; priority: number; packageId?: string; /** @@ -710,6 +726,122 @@ export interface HookEntry { hookName?: string; } +/** `object` / `excludeObjects` in either accepted spelling → a name list. */ +function hookTargetList(target: string | string[] | undefined): string[] { + if (target === undefined) return []; + return Array.isArray(target) ? target : [target]; +} + +/** + * [#5928] Does `entry` cover `objectName`? **The** answer — one function, both + * consumers. + * + * ## Why this is a function and not two `if`s + * + * `triggerHooks` (dispatch) and `hasHooksFor` (the #5038 / #5284 bulk-write + * gate that decides whether the matched row set is worth reading) used to carry + * this rule as two hand-written copies of one semantic. `hasHooksFor`'s own + * comment named the hazard that arrangement carries: a gate LOOSER than the + * dispatch only wastes a query, but a gate TIGHTER than the dispatch silently + * drops hooks that were going to fire. Two copies stay in agreement only for as + * long as everyone who edits one remembers the other, and #5928 adds a second + * dimension to the rule — exactly the edit that would have desynchronised them. + * So the copies are gone: both consumers call this, and the property test in + * `hook-exclude-objects.test.ts` pins the implication (`dispatched ⇒ gate open`) + * over the full allow × exclude matrix rather than trusting the structure. + * + * ## The semantic + * + * `matches = allowMatches && !excludeMatches`, where an absent `object` admits + * everything (global) and `'*'` in `object` does the same explicitly. + * `excludeObjects` names are matched literally — `'*'` is refused at + * registration, so it can never reach this as a subtract-everything wildcard. + * + * The allow half keeps the TRUTHINESS test both copies used verbatim, rather + * than the `!== undefined` that reads more precisely. They differ on exactly one + * input, `object: ''`, which today registers a GLOBAL hook (falsy ⇒ no filter) — + * #4281's failure mode surviving on the code path it never covered, since it + * closed the metadata path at the schema and the binder. Flipping it here would + * turn a hook that fires on everything into one that fires on nothing, silently, + * inside a PR about a different face of the contract. Out of scope by + * construction: preserved, pinned in `hook-exclude-objects.test.ts`, and filed + * separately. + */ +export function hookMatchesObject( + entry: Pick, + objectName: string, +): boolean { + // Allow half: absent = global; otherwise the wildcard or a literal name. + if (entry.object) { + const targets = hookTargetList(entry.object); + if (!targets.includes('*') && !targets.includes(objectName)) return false; + } + // Subtract half: any literal hit removes the object from the admitted set. + if (entry.excludeObjects) { + if (hookTargetList(entry.excludeObjects).includes(objectName)) return false; + } + return true; +} + +/** + * [#5928] Registration-time refusal for the `excludeObjects` face, following + * the sister ruling on empty hook targets (#4281 / #4001, ADR-0078 "no silently + * inert declaration"). + * + * Two shapes are refused, both statically decidable at the call site: + * + * - **`''` / `['']`** (or any blank member) — no object is named `''`, so the + * entry subtracts nothing while reading as though it subtracts something. + * Inert, and inert in the direction that quietly keeps a hook firing on + * objects its author believed they had excluded. + * - **`'*'` anywhere in the list** — subtracting the full set from any allow + * set leaves the empty set: a hook that can never fire, registered + * "successfully". That is ADR-0078's silently-inert declaration exactly, and + * the same never-fire shape #4281 refused when `['']` produced it from the + * other side. + * + * `[]` is deliberately ACCEPTED. It is the honest spelling of "subtract + * nothing" — identical in meaning to omitting the key, and the natural value of + * a spread (`excludeObjects: [...SKIP_OBJECTS]`) whose source list is empty. + * The refusal above is not "empty is bad"; it is "a name that matches nothing" + * and "a name that matches everything". #4281 refused `[]` on `object` for a + * reason that does not exist here: there, the binder WIDENED the empty target + * into `'*'`, so blank intent silently became the maximum blast radius. Nothing + * transforms this value. + * + * A throw, not a warn: `excludeObjects` ships in this release with no callers, + * so strictness costs no migration, and unlike the unknown-event branch above + * (where a custom driver dispatching its own events is a legitimate reading) + * neither refused shape has one. + */ +function assertValidHookExcludeObjects( + excludeObjects: string | string[] | undefined, + event: string, +): void { + if (excludeObjects === undefined) return; + const names = hookTargetList(excludeObjects); + for (const name of names) { + if (typeof name !== 'string' || name.trim().length === 0) { + throw new Error( + `[ObjectQL] Hook '${event}' declares an empty \`excludeObjects\` entry. ` + + 'No object is named \'\', so it would subtract nothing while reading as if it ' + + 'subtracted something — a hook still firing on objects it appears to exclude. ' + + 'Name the object(s) to exclude — `excludeObjects: [\'sys_audit_log\']` — or omit ' + + 'the option entirely (an empty array is accepted and subtracts nothing).', + ); + } + if (name === '*') { + throw new Error( + `[ObjectQL] Hook '${event}' excludes the wildcard '*', which subtracts every ` + + 'object and leaves a hook that can never fire (ADR-0078: no silently inert ' + + 'declaration). Exclude the object names you mean — ' + + '`excludeObjects: [\'sys_audit_log\']` — or, if the hook really should not be ' + + 'registered, do not register it.', + ); + } + } +} + /** Function registry entry — see `registerFunction`. */ export interface FunctionEntry { handler: HookHandler; @@ -1142,10 +1274,17 @@ export class ObjectQL implements IObjectQLEngine { * Register a hook * @param event The event name (e.g. 'beforeFind', 'afterInsert') * @param handler The handler function - * @param options Optional: target object(s) and priority + * @param options Optional: target object(s), objects to exclude, and priority */ registerHook(event: string, handler: HookHandler, options?: { object?: string | string[]; + /** + * [#5928] Object name(s) subtracted from what `object` admits — the way to + * say "global, except these". See {@link HookEntry.excludeObjects} for why + * an allow list cannot express it, and + * {@link assertValidHookExcludeObjects} for the two refused shapes. + */ + excludeObjects?: string | string[]; priority?: number; packageId?: string; /** Original metadata Hook definition (set by `bindHooksToEngine`). */ @@ -1153,6 +1292,9 @@ export class ObjectQL implements IObjectQLEngine { /** Stable name from metadata (set by `bindHooksToEngine`). */ hookName?: string; }) { + // [#5928] Refuse an exclusion face that subtracts nothing (`''`) or + // everything (`'*'`) before anything is registered or reported. + assertValidHookExcludeObjects(options?.excludeObjects, event); // [#3195] Guard against enum-vs-dispatch drift: a hook on an event the // engine never triggers would register "successfully" and then silently // never fire. Warn loudly rather than swallow it. Not a hard reject — a @@ -1172,6 +1314,7 @@ export class ObjectQL implements IObjectQLEngine { entries.push({ handler, object: options?.object, + excludeObjects: options?.excludeObjects, priority: options?.priority ?? 100, packageId: options?.packageId, meta: options?.meta, @@ -1179,7 +1322,10 @@ export class ObjectQL implements IObjectQLEngine { }); // Sort by priority (lower runs first) entries.sort((a, b) => a.priority - b.priority); - this.logger.debug('Registered hook', { event, object: options?.object, priority: options?.priority ?? 100, totalHandlers: entries.length }); + // The exclusion face is reported alongside the allow face: a registration's + // scope is BOTH halves (#5928), and a log that printed only `object` would + // describe a global hook for an entry that is global-minus-twenty-tables. + this.logger.debug('Registered hook', { event, object: options?.object, excludeObjects: options?.excludeObjects, priority: options?.priority ?? 100, totalHandlers: entries.length }); } /** @@ -1409,12 +1555,10 @@ export class ObjectQL implements IObjectQLEngine { (context.session as { skipAutomations?: boolean } | undefined)?.skipAutomations === true; for (const entry of entries) { - // Per-object matching - if (entry.object) { - const targets = Array.isArray(entry.object) ? entry.object : [entry.object]; - if (!targets.includes('*') && !targets.includes(context.object)) { - continue; // Skip non-matching hooks - } + // Per-object matching — allow face minus exclusion face, the one shared + // rule `hasHooksFor` also reads (#5928). + if (!hookMatchesObject(entry, context.object)) { + continue; // Skip non-matching hooks } if (skipAutomations && entry.meta) { this.logger.debug('Skipping metadata-bound hook (skipAutomations)', { event, hook: entry.hookName }); @@ -1427,26 +1571,26 @@ export class ObjectQL implements IObjectQLEngine { /** * [#5038] Would `triggerHooks(event, ctx)` reach ANY handler for `object`? * - * Mirrors the per-object filter in `triggerHooks` exactly (an entry with no - * `object` is global; an array or `'*'` widens it), because this answer gates - * a READ of the whole matched row set on the bulk write path. Getting it - * looser than the dispatch loop only costs a wasted query; getting it - * TIGHTER would silently drop hooks that were going to fire, so the two must - * be read together. + * Applies the SAME per-object rule the dispatch loop applies — literally the + * same function, {@link hookMatchesObject}, since #5928 — because this answer + * gates a READ of the whole matched row set on the bulk write path. Getting + * it looser than the dispatch loop only costs a wasted query; getting it + * TIGHTER would silently drop hooks that were going to fire. Until #5928 the + * two were separate hand-written copies of one semantic and this comment + * asked the reader to keep them in step; sharing the function is what + * actually keeps them in step, and the property test in + * `hook-exclude-objects.test.ts` pins the direction that matters. * * `session.skipAutomations` is deliberately NOT consulted: it suppresses only * metadata-bound entries, and code-registered hooks (audit, sharing) still * run, so the row set is still needed. Over-reading in that case is a cost, - * never a correctness loss. + * never a correctness loss — and it is the SAFE side of the asymmetry above, + * which is why the pin asserts an implication rather than an equality. */ private hasHooksFor(event: string, object: string): boolean { const entries = this.hooks.get(event); if (!entries || entries.length === 0) return false; - return entries.some((entry) => { - if (!entry.object) return true; - const targets = Array.isArray(entry.object) ? entry.object : [entry.object]; - return targets.includes('*') || targets.includes(object); - }); + return entries.some((entry) => hookMatchesObject(entry, object)); } /** diff --git a/packages/objectql/src/hook-exclude-objects.test.ts b/packages/objectql/src/hook-exclude-objects.test.ts new file mode 100644 index 0000000000..590f4a4907 --- /dev/null +++ b/packages/objectql/src/hook-exclude-objects.test.ts @@ -0,0 +1,354 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#5928] The hook registration contract can express "global, EXCEPT these + * objects". + * + * ## What was missing + * + * `registerHook` options carried one scope face: `object`, an ALLOW list (absent + * = global, `'*'` = every object). An allow list and a deny list are + * interchangeable only over a CLOSED universe of object names, and this one is + * open — `applyObjectRegistryMutation` registers new objects into a running + * engine on a successful `/meta` PUT, and `SchemaRegistry.registerObject` emits + * no event a plugin could subscribe to. So a registrant that wanted "everything + * except these twenty platform tables" had exactly two options, both wrong: + * + * - keep the knowledge inside the handler as an early return, which leaves the + * registration GLOBAL — so the #5038 / #5284 per-object gates, which read the + * registration face, answer "yes, hooks apply" for objects the handler is + * about to skip, and the bulk-write path pays for a row-set read nobody uses; + * - enumerate the complement into `object`, which freezes the list at boot — + * the "probe D" scenario below, where an object created afterwards is + * silently NOT covered. For the audit plugin that is a compliance regression + * that reports nothing. + * + * `excludeObjects` is the third option: the deny half, declared on the + * registration where the engine reads it. + * + * ## What these tests pin + * + * 1. the matching matrix — allow face × exclusion face, both spellings; + * 2. probe D — under an exclusion face, an object nobody enumerated is covered + * BY DEFAULT, which is the whole reason this shape was chosen over + * enumerating the complement; + * 3. the two refused shapes (`''`/`['']`, `'*'`), following #4281's ruling on + * empty hook targets, plus the `[]` that is deliberately accepted; + * 4. **the property**: the `hasHooksFor` gate is never TIGHTER than the + * `triggerHooks` dispatch. Both now call one `hookMatchesObject`, so this is + * structurally true today — the test is what keeps it true, and it asserts + * the implication rather than an equality because the gate is allowed to be + * looser (it deliberately ignores `skipAutomations`). + */ + +import { describe, it, expect, vi } from 'vitest'; +import { ObjectQL, hookMatchesObject, type HookEntry } from './engine.js'; + +const EVENT = 'afterUpdate'; + +function makeLogger() { + return { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }; +} + +function makeEngine() { + return new ObjectQL({ logger: makeLogger() }); +} + +/** Dispatch `EVENT` for `object` and report which handler names ran. */ +async function dispatch(engine: ObjectQL, object: string): Promise { + const fired: string[] = []; + (engine as any)._firedSink = fired; + await engine.triggerHooks(EVENT, { object, event: EVENT, input: {} } as any); + return fired; +} + +/** Register a handler that records `name` into the engine's dispatch sink. */ +function register( + engine: ObjectQL, + name: string, + options: { object?: string | string[]; excludeObjects?: string | string[] }, +) { + engine.registerHook( + EVENT, + () => { + ((engine as any)._firedSink as string[]).push(name); + }, + options, + ); +} + +const gateOpen = (engine: ObjectQL, object: string): boolean => + (engine as any).hasHooksFor(EVENT, object); + +describe('[#5928] hook `excludeObjects` — matching semantics', () => { + it('global minus a list: fires on unlisted objects, not on excluded ones', async () => { + const engine = makeEngine(); + register(engine, 'audit', { excludeObjects: ['sys_audit_log', 'sys_job'] }); + + expect(await dispatch(engine, 'account')).toEqual(['audit']); + expect(await dispatch(engine, 'sys_audit_log')).toEqual([]); + expect(await dispatch(engine, 'sys_job')).toEqual([]); + }); + + it('explicit wildcard minus a list behaves the same as the implicit global', async () => { + const engine = makeEngine(); + register(engine, 'starred', { object: '*', excludeObjects: ['sys_audit_log'] }); + + expect(await dispatch(engine, 'account')).toEqual(['starred']); + expect(await dispatch(engine, 'sys_audit_log')).toEqual([]); + }); + + it('subtracts from a finite allow list too, not only from the global set', async () => { + const engine = makeEngine(); + register(engine, 'pair', { + object: ['account', 'contact'], + excludeObjects: ['contact'], + }); + + expect(await dispatch(engine, 'account')).toEqual(['pair']); + expect(await dispatch(engine, 'contact')).toEqual([]); + // Outside the allow list: the allow face already refused it. + expect(await dispatch(engine, 'lead')).toEqual([]); + }); + + it('accepts the bare-string spelling on both faces', async () => { + const engine = makeEngine(); + register(engine, 'single', { object: '*', excludeObjects: 'sys_audit_log' }); + + expect(await dispatch(engine, 'account')).toEqual(['single']); + expect(await dispatch(engine, 'sys_audit_log')).toEqual([]); + }); + + it('leaves the pre-existing allow-only registrations untouched', async () => { + const engine = makeEngine(); + register(engine, 'global', {}); + register(engine, 'allowOne', { object: 'account' }); + register(engine, 'allowMany', { object: ['account', 'contact'] }); + register(engine, 'wildcard', { object: '*' }); + + expect((await dispatch(engine, 'account')).sort()) + .toEqual(['allowMany', 'allowOne', 'global', 'wildcard']); + expect((await dispatch(engine, 'lead')).sort()).toEqual(['global', 'wildcard']); + }); + + it('`excludeObjects: []` is a no-op — the honest spelling of "subtract nothing"', async () => { + const engine = makeEngine(); + register(engine, 'spread', { excludeObjects: [] }); + + // The natural value of `excludeObjects: [...SKIP_OBJECTS]` when the source + // list is empty. Nothing transforms it, so it means what it says — unlike + // #4281's `object: []`, which the binder widened into the wildcard. + expect(await dispatch(engine, 'account')).toEqual(['spread']); + expect(await dispatch(engine, 'sys_audit_log')).toEqual(['spread']); + }); +}); + +describe('[#5928] probe D — an object registered after the hook was', () => { + /* + * The decisive comparison. `late_object` stands for an object a `/meta` PUT + * registers into a live engine after the plugin's `init()` ran — the case that + * ruled out enumerating the complement. + */ + it('is covered by default under an exclusion face', async () => { + const engine = makeEngine(); + register(engine, 'audit', { excludeObjects: ['sys_audit_log'] }); + + expect(await dispatch(engine, 'late_object')).toEqual(['audit']); + expect(gateOpen(engine, 'late_object')).toBe(true); + }); + + it('is silently NOT covered under an enumerated allow list', async () => { + const engine = makeEngine(); + // The same intent expressed as the complement, frozen at registration time. + register(engine, 'audit', { object: ['account', 'contact'] }); + + expect(await dispatch(engine, 'late_object')).toEqual([]); + expect(gateOpen(engine, 'late_object')).toBe(false); + }); +}); + +describe('[#5928] refused exclusion faces (#4281 / ADR-0078 lineage)', () => { + it('refuses the empty string — it subtracts nothing while reading as if it did', () => { + const engine = makeEngine(); + expect(() => engine.registerHook(EVENT, () => {}, { excludeObjects: '' })) + .toThrow(/empty `excludeObjects` entry/); + }); + + it("refuses `['']`, including as one member of an otherwise valid list", () => { + const engine = makeEngine(); + expect(() => engine.registerHook(EVENT, () => {}, { excludeObjects: [''] })) + .toThrow(/empty `excludeObjects` entry/); + expect(() => engine.registerHook(EVENT, () => {}, { excludeObjects: ['sys_job', ' '] })) + .toThrow(/empty `excludeObjects` entry/); + }); + + it("refuses `'*'` — subtracting every object leaves a hook that can never fire", () => { + const engine = makeEngine(); + expect(() => engine.registerHook(EVENT, () => {}, { excludeObjects: '*' })) + .toThrow(/can never fire/); + expect(() => engine.registerHook(EVENT, () => {}, { excludeObjects: ['sys_job', '*'] })) + .toThrow(/ADR-0078/); + }); + + it('registers nothing when it refuses', async () => { + const engine = makeEngine(); + expect(() => engine.registerHook(EVENT, () => {}, { excludeObjects: '*' })).toThrow(); + + // A refusal that still pushed the entry would leave exactly the inert + // registration the refusal exists to prevent. + expect(await dispatch(engine, 'account')).toEqual([]); + expect(gateOpen(engine, 'account')).toBe(false); + }); + + it('names the fix in the message rather than only the fault', () => { + const engine = makeEngine(); + const empty = (() => { + try { engine.registerHook(EVENT, () => {}, { excludeObjects: '' }); return ''; } + catch (e) { return (e as Error).message; } + })(); + expect(empty).toContain("excludeObjects: ['sys_audit_log']"); + expect(empty).toContain('an empty array is accepted'); + }); +}); + +describe('[#5928] registration log reports the exclusion face', () => { + it("carries `excludeObjects` in the 'Registered hook' debug record", () => { + const logger = makeLogger(); + const engine = new ObjectQL({ logger }); + engine.registerHook(EVENT, () => {}, { excludeObjects: ['sys_audit_log', 'sys_job'] }); + + const record = logger.debug.mock.calls.find((c) => c[0] === 'Registered hook'); + expect(record).toBeDefined(); + // A log printing only `object` would describe this entry as a plain global + // hook, which is precisely what it is not. + expect(record![1]).toMatchObject({ + event: EVENT, + excludeObjects: ['sys_audit_log', 'sys_job'], + }); + }); +}); + +/* + * ── The property: the gate is never tighter than the dispatch ─────────────── + * + * `hasHooksFor` gates a READ of the whole matched row set on the bulk write + * path (#5038, #5284). The asymmetry its comment records is the reason this is + * a property test and not a spot check: + * + * gate LOOSER than dispatch → a wasted query. Cost. + * gate TIGHTER than dispatch → hooks that were going to fire are silently + * dropped. Correctness, and silent. + * + * So the assertion is the implication `dispatched ⇒ gate open`, over every + * combination of the two scope faces — never an equality, which would forbid + * the legitimate looseness (`hasHooksFor` deliberately ignores + * `skipAutomations`). + */ +describe('[#5928] property: hasHooksFor is never tighter than triggerHooks', () => { + const SCOPES: Array<{ object?: string | string[]; excludeObjects?: string | string[] }> = [ + {}, + { object: '*' }, + { object: 'account' }, + { object: ['account', 'contact'] }, + { excludeObjects: 'account' }, + { excludeObjects: ['account', 'sys_job'] }, + { object: '*', excludeObjects: 'account' }, + { object: '*', excludeObjects: ['account', 'sys_job'] }, + { object: ['account', 'contact'], excludeObjects: 'contact' }, + { object: ['account', 'contact'], excludeObjects: ['account', 'contact'] }, + { object: 'account', excludeObjects: 'lead' }, + { excludeObjects: [] }, + ]; + const OBJECTS = ['account', 'contact', 'lead', 'sys_job', 'late_object']; + + it('holds for every single-entry scope × object pair', async () => { + for (const scope of SCOPES) { + for (const object of OBJECTS) { + const engine = makeEngine(); + register(engine, 'h', scope); + + const dispatched = (await dispatch(engine, object)).length > 0; + const open = gateOpen(engine, object); + + if (dispatched && !open) { + throw new Error( + `gate TIGHTER than dispatch for ${JSON.stringify(scope)} on '${object}': ` + + 'a hook fired that hasHooksFor said could not', + ); + } + // With one shared matcher and a single entry the two agree exactly; + // asserting that here would catch a drift the implication tolerates. + expect(open).toBe(dispatched); + } + } + }); + + /* + * Realism, not detection power. Measured against a deliberately broken build + * (gate applies the exclusion, dispatch reads the allow face only) the + * single-entry matrix above fails with the offending scope named, while THIS + * case still passes: once a global entry is in the set, the gate is open for + * every object, so no violation can surface. Kept because coexisting + * registrations are the real deployment shape — just do not read it as the + * assertion with teeth. + */ + it('holds for the whole scope set registered at once', async () => { + const engine = makeEngine(); + SCOPES.forEach((scope, i) => register(engine, `h${i}`, scope)); + + for (const object of OBJECTS) { + const dispatched = (await dispatch(engine, object)).length > 0; + expect(gateOpen(engine, object) || !dispatched).toBe(true); + } + }); + + it('holds when `skipAutomations` suppresses metadata-bound entries', async () => { + // The one legal direction of disagreement: the gate stays open while the + // dispatch suppresses. Loose, never tight. + const engine = makeEngine(); + engine.registerHook(EVENT, () => { throw new Error('must not run'); }, { + excludeObjects: ['sys_job'], + meta: { name: 'metadata_bound' }, + }); + + await engine.triggerHooks(EVENT, { + object: 'account', + event: EVENT, + input: {}, + session: { skipAutomations: true }, + } as any); + + expect(gateOpen(engine, 'account')).toBe(true); + expect(gateOpen(engine, 'sys_job')).toBe(false); + }); +}); + +describe('[#5928] hookMatchesObject — the one shared rule', () => { + it('is the function both consumers read', () => { + // Guards against a future edit re-introducing a hand-written copy: if this + // rule ever disagrees with either consumer, the property tests above fail. + const entry: Pick = { + object: '*', + excludeObjects: ['sys_job'], + }; + expect(hookMatchesObject(entry, 'account')).toBe(true); + expect(hookMatchesObject(entry, 'sys_job')).toBe(false); + expect(hookMatchesObject({}, 'anything')).toBe(true); + expect(hookMatchesObject({ object: [] }, 'account')).toBe(false); + }); + + it('preserves the pre-existing `object: \'\'` reading unchanged (out of scope)', async () => { + // Both replaced copies tested `object` for TRUTHINESS, so `''` registers a + // GLOBAL hook — #4281's "blank intent becomes the broadest blast radius", + // surviving on the code path that ruling did not cover (it closed the + // authorable schema and the binder). Flipping it inside this PR would + // silently convert a fires-on-everything hook into a fires-on-nothing one. + // Pinned as UNCHANGED, not as correct; filed separately. + expect(hookMatchesObject({ object: '' }, 'account')).toBe(true); + + const engine = makeEngine(); + register(engine, 'blank', { object: '' }); + expect(await dispatch(engine, 'account')).toEqual(['blank']); + expect(gateOpen(engine, 'account')).toBe(true); + }); +}); diff --git a/packages/spec/src/contracts/objectql-engine-hook-scope.test.ts b/packages/spec/src/contracts/objectql-engine-hook-scope.test.ts new file mode 100644 index 0000000000..4408385576 --- /dev/null +++ b/packages/spec/src/contracts/objectql-engine-hook-scope.test.ts @@ -0,0 +1,97 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#5928] The `objectql` slot's hook-registration contract carries BOTH scope + * faces. + * + * `IObjectQLEngine.registerHook` is the declaration consumers outside + * `packages/objectql` program against — it is how a plugin knows what it may + * pass without reaching for `any`. Until #5928 it declared one face, `object`, + * an allow list; "global, except these objects" had no spelling, which is what + * blocked the audit plugin from moving its skip list out of the handler and onto + * the registration where the #5038 / #5284 per-object gates can read it. + * + * The declaration and the engine that implements it move in ONE PR on purpose: + * shipping this half alone would leave `packages/spec` advertising an option the + * engine ignores — a declared-but-unenforced window, the failure this repo keeps + * paying to close. + * + * The behaviour itself (`matches = allowMatches && !excludeMatches`, the refused + * shapes, the gate-vs-dispatch property) is pinned in + * `packages/objectql/src/hook-exclude-objects.test.ts`: only objectql can + * execute a dispatch, and spec must not depend on it. What is assertable HERE is + * the shape of the contract, so that is all this file claims. + */ + +import { describe, it, expect } from 'vitest'; +import type { IObjectQLEngine } from './objectql-engine'; + +/** + * Exact type equality. Written as a VALUE assertion below (`const x: Equals<…> = + * true`) rather than the bare `type _X = Expect<…>` alias the older contract + * tests use: an unused type alias is a `noUnusedLocals` error under + * `tsconfig.test.json`, which is why those files carry measured entries in + * `test-typecheck-debt.json`. That ledger is shrink-only, so a new file earns + * its way in by having no errors at all. + */ +type Equals = (() => G extends A ? 1 : 2) extends (() => G extends B ? 1 : 2) ? true : false; + +type RegisterHookOptions = NonNullable[2]>; + +describe('[#5928] IObjectQLEngine.registerHook scope faces', () => { + it('declares `excludeObjects` in both accepted spellings', () => { + // Fails to COMPILE if the declared type is anything else — `true` is not + // assignable to `false`. + const declared: Equals< + RegisterHookOptions['excludeObjects'], string | string[] | undefined + > = true; + // Mirrors `object` exactly — one vocabulary for both faces, so a caller + // never has to remember that one takes a list and the other a single name. + const mirrorsAllowFace: Equals< + RegisterHookOptions['excludeObjects'], RegisterHookOptions['object'] + > = true; + + expect(declared).toBe(true); + expect(mirrorsAllowFace).toBe(true); + }); + + it('keeps the option optional — an allow-only registration stays legal', () => { + // Every registration that compiled before #5928 still compiles: the face is + // additive, and absent means "subtract nothing". + const allowOnly: RegisterHookOptions = { object: 'account', priority: 50 }; + const global: RegisterHookOptions = {}; + expect(allowOnly.excludeObjects).toBeUndefined(); + expect(global.excludeObjects).toBeUndefined(); + }); + + it('admits the shape the audit plugin needs — global minus a static list', () => { + // The registration #5860 is blocked on: no `object`, so global, minus the + // platform tables the writer must not recurse into. + const auditScope: RegisterHookOptions = { + excludeObjects: ['sys_audit_log', 'sys_job', 'sys_metadata'], + packageId: 'plugin-audit', + }; + expect(auditScope.excludeObjects).toHaveLength(3); + + // …and the explicit-wildcard spelling of the same intent. + const starred: RegisterHookOptions = { object: '*', excludeObjects: 'sys_audit_log' }; + expect(starred.excludeObjects).toBe('sys_audit_log'); + }); + + it('is satisfiable by an implementation typed against the contract', () => { + // A structural check that the widened options do not break implementors: + // this is how a host or a test double declares the member. + const calls: Array<{ event: string; excludeObjects?: string | string[] }> = []; + const registerHook: IObjectQLEngine['registerHook'] = (event, _handler, options) => { + calls.push({ event, excludeObjects: options?.excludeObjects }); + }; + + registerHook('afterUpdate', () => {}, { excludeObjects: ['sys_audit_log'] }); + registerHook('afterInsert', () => {}); + + expect(calls).toEqual([ + { event: 'afterUpdate', excludeObjects: ['sys_audit_log'] }, + { event: 'afterInsert', excludeObjects: undefined }, + ]); + }); +}); diff --git a/packages/spec/src/contracts/objectql-engine.ts b/packages/spec/src/contracts/objectql-engine.ts index 928c1de04d..67c2319ce6 100644 --- a/packages/spec/src/contracts/objectql-engine.ts +++ b/packages/spec/src/contracts/objectql-engine.ts @@ -184,10 +184,37 @@ export interface IObjectQLEngine extends IDataEngine { executeAction(objectName: string, actionName: string, ctx: any): Promise; // ── Hook / middleware seams ────────────────────────────────────────── + /** + * Register a code-path hook. + * + * `object` is an ALLOW list (absent = global, `'*'` = every object); + * `excludeObjects` subtracts from whatever that admits, so the scope a + * registration expresses is + * `matches(entry, X) = allowMatches(entry, X) && !excludeMatches(entry, X)`. + * + * The subtraction half exists because an allow list cannot express "global, + * except these" over an OPEN universe (#5928): `/meta` PUT registers new + * objects into a live engine, so a registrant that enumerated the complement + * of its skip list would silently stop covering every object created after + * boot — the compliance-relevant direction of that failure is what ruled the + * enumerate-the-complement option out. Declared here rather than left to a + * predicate callback so the scope stays static, printable in the + * registration log, and introspectable. + * + * NOT mirrored onto the authorable `HookSchema` (`data/hook.zod.ts`): the + * consumer is plugin code, and no metadata author needs "global minus a + * list" today. + */ registerHook( event: string, handler: (context: any) => Promise | void, - options?: { object?: string | string[]; priority?: number; packageId?: string }, + options?: { + object?: string | string[]; + /** Object name(s) subtracted from `object`'s admitted set (#5928). */ + excludeObjects?: string | string[]; + priority?: number; + packageId?: string; + }, ): void; unregisterHooksByPackage(packageId: string): number; /**