Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions .changeset/21604-hook-handler-package-scope.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
---
'@objectstack/objectql': minor
'@objectstack/spec': minor
---

fix(objectql,spec)!: a hook's `handler` name resolves inside the hook's own package only (#21604)

Clause-②: yes (narrowing)

<!-- adr-0087: not-required (no-migration-prescription) no authorable key, spelling, export of a published release or stored shape moves: `HookSchema`'s shape is unchanged (only `HookSchema.handler`'s doc changes), so `objectstack migrate meta` has nothing to rewrite. What changes is which function a string `handler` may bind to at registration. Census of compositions relying on cross-package resolution by name, at the claim: zero in objectstack `examples/**` (15fe567c9c, whose only string `handler` is a job's), hotcrm (f24c196588) and objectos (7612ffebd1); this repository commits no `--artifact` runtime module; cloud is NOT MEASURED (unreachable from the claim's session). The other categories are closed on facts: both packages publish (not `unpublished`); no ADR-0087 id covers a binding rule (not `registered` / `already-registered`); and the change is runtime behaviour, not a declaration (not `runtime-interface-only` / `type-surface-only`). -->

**BREAKING**: a hook whose `handler` is a function NAME (the deprecated form, `handler: 'my_fn'`, with no `body`) now binds only to a function its own package holds. It used to fall back to the engine-wide function registry, which is keyed by bare name, so the hook could bind to a function another package registered under the same name and run that package's code on its own events.

- **Accepted before:** a string `handler` resolved against the functions handed to the hook's bind, then against every function any package had registered on the engine. A name found nowhere was skipped with a `warn`.
- **Accepted now:** a string `handler` resolves against the functions handed to the hook's bind (the package's `functions`, which an `--artifact` runtime module supplies), then against the functions the same package (`packageId`) registered on the engine. Nothing else.
- **Refused now, at registration:** a name the hook's own package does not hold, whether another package registered it or nobody did. The hook is not bound. The refusal carries `INVALID_REFERENCE` with status `400` (ADR-0112), names the hook, the function and the package, and is recorded on the bind result (`BindHooksResult.errors[]` gains `code` and `status`) and logged at `error`. Under `strict` (`OBJECTQL_STRICT_HOOKS=1`) it is thrown.
- **The doors:** a hook authored at runtime through the metadata API (`PUT /api/v1/meta/hook/:name`) ships with no code package and holds no functions, so a `handler`-only hook authored there is refused when the door binds it; the save itself still answers as before. In a composition of several apps, one app's hook can no longer bind to another app's function. A bind that names no owning package (direct `bindHooksToEngine` use without `packageId`) resolves only the functions handed to it.
- **Unchanged:** a hook with a `body` binds as before. An app's hook naming its own `defineStack({ functions })` entry, or a function its own `--artifact` runtime module exports, binds as before. The install-local door's refusal of a hook with no `body` is unchanged.

What to do with a refused hook: give it a `body` (sandboxed JS), or declare the function in the hook's own package's `functions`. To reuse another package's function, import it from the package that owns it and declare it there. This ships as `minor`, under the launch-window convention for narrowings of an accept set.
12 changes: 7 additions & 5 deletions packages/objectql/src/engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3657,10 +3657,11 @@ export class ObjectQL implements IObjectQLEngine {
*/
private readonly actionActivation = new ActionActivationProjection();

// Function registry: name → handler. Used by `bindHooksToEngine` to
// resolve string-named hook handlers (the JSON-safe form). Populated by
// `defineStack({ functions })` via `AppPlugin`, or directly via
// `engine.registerFunction(...)`.
// Function registry: name → handler, each entry stamped with its owning
// package. Used by `bindHooksToEngine` to resolve string-named hook
// handlers (the JSON-safe form) — only against entries the hook's OWN
// package registered. Populated by `defineStack({ functions })` via
// `AppPlugin`, or directly via `engine.registerFunction(...)`.
private functions = new Map<string, FunctionEntry>();

// Realtime service for event publishing
Expand Down Expand Up @@ -3867,7 +3868,8 @@ export class ObjectQL implements IObjectQLEngine {
* string from a `Hook.handler` field, an `Action.target`, or a flow
* `script` node's `config.function`. This is the JSON-safe form of
* handler binding — declarative metadata persisted to disk or shipped
* over the wire only carries the name.
* over the wire only carries the name. A `Hook.handler` reaches the entry
* only from a hook of the same `packageId` (`bindHooksToEngine`).
*
* The third parameter accepts either the owning `packageId` (its original
* shape, unchanged for every existing caller) or a
Expand Down
195 changes: 195 additions & 0 deletions packages/objectql/src/hook-binder-package-scope.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,195 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* A hook's `handler` name resolves inside the hook's OWN package only.
*
* The engine's function registry is keyed by bare name, and the binder used to
* fall back to it unscoped: a hook naming `shared_stamp` bound to whichever
* package had registered a function of that name, so that package's code ran on
* this package's events. Now a name resolves against the functions handed to
* the hook's own bind, then against the entries the SAME package registered; a
* name the package does not hold is refused at registration with the ADR-0112
* envelope (`INVALID_REFERENCE`, 400) and the hook is not bound.
*
* Every refusal here asserts the code, the status and that the hook did not
* bind (the other package's function never runs on the event). The controls
* are the two shapes a package's own functions take: the functions handed to
* the same bind, and a function the same package registered in an earlier bind.
*/

import { describe, it, expect, vi } from 'vitest';
import { ObjectQL } from './engine.js';
import {
bindHooksToEngine,
HOOK_HANDLER_NOT_IN_PACKAGE_CODE,
HOOK_HANDLER_NOT_IN_PACKAGE_STATUS,
} from './hook-binder.js';
import type { Hook, HookContext } from '@objectstack/spec/data';

function captureLogger() {
const logger: any = {
debug: vi.fn(),
info: vi.fn(),
warn: vi.fn(),
error: vi.fn(),
trace: vi.fn(),
fatal: vi.fn(),
};
logger.child = () => logger;
return logger;
}

function makeEngine(logger = captureLogger()) {
return { engine: new ObjectQL({ logger }), logger };
}

function ctxFor(object = 'account'): HookContext {
return { object, event: 'beforeInsert', input: { data: {} }, ql: undefined } as unknown as HookContext;
}

const hookNaming = (name: string, handler: string): Hook => ({
name,
object: 'account',
events: ['beforeInsert'],
priority: 100,
handler,
});

/** Package A registers `shared_stamp` the way a code package does: through its own bind's `functions`. */
function registerPackageA(engine: ObjectQL, ran: string[]) {
bindHooksToEngine(engine, [], {
packageId: 'app:com.example.a',
functions: { shared_stamp: async () => { ran.push('a:shared_stamp'); } },
});
}

describe('a hook handler name resolves inside its own package only', () => {
it('the envelope constants are the standard catalog member and its status', () => {
expect(HOOK_HANDLER_NOT_IN_PACKAGE_CODE).toBe('INVALID_REFERENCE');
expect(HOOK_HANDLER_NOT_IN_PACKAGE_STATUS).toBe(400);
});

it('refuses a hook naming a function ANOTHER package registered, and that function never runs', async () => {
const { engine } = makeEngine();
const ran: string[] = [];
registerPackageA(engine, ran);

const result = bindHooksToEngine(engine, [hookNaming('b_cross', 'shared_stamp')], {
packageId: 'app:com.example.b',
});

expect(result.registered).toBe(0);
expect(result.skipped).toBe(1);
expect(result.errors).toHaveLength(1);
expect(result.errors[0]).toMatchObject({ hook: 'b_cross', code: 'INVALID_REFERENCE', status: 400 });
expect(result.errors[0]!.reason).toContain("'shared_stamp'");
expect(result.errors[0]!.reason).toContain("'app:com.example.b'");

await engine.triggerHooks('beforeInsert', ctxFor());
expect(ran, "package A's function ran on package B's event").toEqual([]);
});

it('under strict, the refusal is thrown with its code and status, and nothing binds', async () => {
const { engine } = makeEngine();
const ran: string[] = [];
registerPackageA(engine, ran);

let thrown: any;
try {
bindHooksToEngine(engine, [hookNaming('b_cross_strict', 'shared_stamp')], {
packageId: 'app:com.example.b',
strict: true,
});
} catch (err) {
thrown = err;
}
expect(thrown).toBeInstanceOf(Error);
expect(thrown).toMatchObject({
code: 'INVALID_REFERENCE',
status: 400,
hook: 'b_cross_strict',
handler: 'shared_stamp',
packageId: 'app:com.example.b',
});

await engine.triggerHooks('beforeInsert', ctxFor());
expect(ran).toEqual([]);
});

it('refuses a name no package holds with the same envelope', async () => {
const { engine } = makeEngine();
const result = bindHooksToEngine(engine, [hookNaming('typo_hook', 'shard_stamp')], {
packageId: 'app:com.example.b',
});
expect(result.registered).toBe(0);
expect(result.errors[0]).toMatchObject({ hook: 'typo_hook', code: 'INVALID_REFERENCE', status: 400 });
});

it('the metadata door (owner `metadata-service`) cannot reach a code package\'s function; the refusal is logged at error with its envelope', async () => {
const { engine, logger } = makeEngine();
const ran: string[] = [];
registerPackageA(engine, ran);

// The door the runtime-authored hooks are bound through.
engine.bindHooks([hookNaming('authored_cross', 'shared_stamp')], { packageId: 'metadata-service' });

await engine.triggerHooks('beforeInsert', ctxFor());
expect(ran, 'a runtime-authored hook ran a code package\'s function').toEqual([]);

const refusals = logger.error.mock.calls.filter(
(call: any[]) => call[2]?.hook === 'authored_cross',
);
expect(refusals).toHaveLength(1);
expect(refusals[0][1]).toBeInstanceOf(Error);
expect(refusals[0][2]).toMatchObject({
code: 'INVALID_REFERENCE',
status: 400,
handler: 'shared_stamp',
packageId: 'metadata-service',
});
});

it('a bind that names no owning package resolves only what it was handed — never an unowned engine entry', async () => {
const { engine } = makeEngine();
const ran: string[] = [];
engine.registerFunction('loose_fn', async () => { ran.push('loose_fn'); });

const result = bindHooksToEngine(engine, [hookNaming('unowned_hook', 'loose_fn')], {});
expect(result.registered).toBe(0);
expect(result.errors[0]).toMatchObject({ hook: 'unowned_hook', code: 'INVALID_REFERENCE', status: 400 });

await engine.triggerHooks('beforeInsert', ctxFor());
expect(ran).toEqual([]);
});

it('control: a hook naming a function handed to its own bind binds and runs', async () => {
const { engine } = makeEngine();
const ran: string[] = [];
registerPackageA(engine, ran);

const result = bindHooksToEngine(engine, [hookNaming('b_own', 'b_stamp')], {
packageId: 'app:com.example.b',
functions: { b_stamp: async () => { ran.push('b:b_stamp'); } },
});
expect(result.registered).toBe(1);
expect(result.errors).toEqual([]);

await engine.triggerHooks('beforeInsert', ctxFor());
expect(ran).toEqual(['b:b_stamp']);
});

it('control: a hook naming a function its OWN package registered in an earlier bind binds and runs', async () => {
const { engine } = makeEngine();
const ran: string[] = [];
registerPackageA(engine, ran);

const result = bindHooksToEngine(engine, [hookNaming('a_own_later', 'shared_stamp')], {
packageId: 'app:com.example.a',
});
expect(result.registered).toBe(1);
expect(result.errors).toEqual([]);

await engine.triggerHooks('beforeInsert', ctxFor());
expect(ran).toEqual(['a:shared_stamp']);
});
});
6 changes: 3 additions & 3 deletions packages/objectql/src/hook-binder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ describe('bindHooksToEngine', () => {
expect(seen).toEqual(['called']);
});

it('skips hooks whose string handler cannot be resolved', () => {
it('refuses a hook whose string handler names no function of its package', () => {
const engine = makeEngine();
const hook: Hook = {
name: 'h3',
Expand All @@ -74,7 +74,7 @@ describe('bindHooksToEngine', () => {
const result = bindHooksToEngine(engine, [hook], { packageId: 'p' });
expect(result.registered).toBe(0);
expect(result.skipped).toBe(1);
expect(result.errors[0]?.reason).toMatch(/unknown function/);
expect(result.errors[0]).toMatchObject({ hook: 'h3', code: 'INVALID_REFERENCE', status: 400 });
});

// #4001: `normalizeObjects` used to widen a blank target to `['*']`, the
Expand Down Expand Up @@ -164,7 +164,7 @@ describe('bindHooksToEngine', () => {
};

expect(() => bindHooksToEngine(engine, [hook], { strict: true }))
.toThrow(/unknown function 'no_such_fn'/);
.toThrowError(expect.objectContaining({ code: 'INVALID_REFERENCE', status: 400, handler: 'no_such_fn' }));
});

it('still records-and-continues when strict is off', () => {
Expand Down
Loading
Loading