diff --git a/.changeset/24289-named-hook-handler-ctx-title.md b/.changeset/24289-named-hook-handler-ctx-title.md new file mode 100644 index 00000000000..7023b12348d --- /dev/null +++ b/.changeset/24289-named-hook-handler-ctx-title.md @@ -0,0 +1,13 @@ +--- +'@objectstack/spec': minor +'@objectstack/objectql': minor +'@objectstack/runtime': patch +--- + +fix(objectql,runtime): a hook whose `handler` names a function gets `ctx.title(field?)` in-process, as an inline handler and a `body` do + +Clause-②: yes (widening) + +- **What was wrong.** A hook whose `handler` is a string naming a function ran in-process without `ctx.title`: `defineStack({ functions })` plus a hook naming an entry on a source boot (`@objectstack/verify`'s `bootStack(config)`, `os serve` over a config), and a string `handler` resolved from the artifact's `runtimeModule` (what `objectstack build` ships for a callable it could not lower to a `body`). The engine's binder resolved the name after the runtime had wrapped the inline handlers, so the function was called with the bare engine context and `await ctx.title()` threw `TypeError: ctx.title is not a function`. +- **What changed.** `bindHooksToEngine` (`@objectstack/objectql`) takes a `wrapHandler(handler, hook)` option, applied at bind to every handler a hook runs in-process, after a string `handler` resolves: an inline function, a name from the bind's `functions`, or a name the same package registered on the engine. A `body`'s handler (built by `bodyRunner`) never takes it. `ObjectQL.bindHooks` and the `IObjectQLEngine.bindHooks` contract (`@objectstack/spec`) carry the option. `bindAppArtifactHandlers` (`@objectstack/runtime`) passes the existing `withInProcessHookTitle` through it, in place of wrapping inline handlers before the bind, so the inline and the named handler take one wrap. +- **Unchanged.** A bind that passes no `wrapHandler` binds every handler exactly as resolved, so a host or harness that calls `bindHooks` itself, and a hook registered with `ql.registerHook(...)`, are not handed `ctx.title`. `HookContext` still declares no `title`. diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 7ca9a353fe8..9883e569afc 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -290,7 +290,7 @@ import { deriveViewContainerObject } from '@objectstack/metadata/view-container' // [#20331] The divergent view-container `name` refusal — the ONE judge this // registrar and `os validate` both call. import { viewContainerNameRefusal } from './view-container-name-refusal.js'; -import { bindHooksToEngine } from './hook-binder.js'; +import { bindHooksToEngine, type BindHooksOptions } from './hook-binder.js'; import { declaredHookCondition } from './hook-wrappers.js'; import { validateRecord, validateRecordInScope, normalizeMultiValueFields, normalizeBlankTypedValues, normalizeNumericStringValues, coerceBooleanFields, ValidationError, buildFieldError, resolveFieldLabel, valueShapePostureSetByEnv, mediaPostureSetByEnv, isScannableValueShapeField, valueShapeStrictEffective, mediaStrictEffective } from './validation/record-validator.js'; import type { AdmittedValueShapeViolation, AdmittedValueShapeViolationSink } from './validation/record-validator.js'; @@ -4866,6 +4866,8 @@ export class ObjectQL implements IObjectQLEngine { strict?: boolean; warnLegacyHandler?: boolean; metrics?: any; + /** Wraps each in-process handler after its name resolves — see {@link BindHooksOptions.wrapHandler}. */ + wrapHandler?: BindHooksOptions['wrapHandler']; }): void { const merged = { ...(opts ?? {}), logger: this.logger } as any; if (!merged.bodyRunner && this._defaultBodyRunner) { diff --git a/packages/objectql/src/hook-binder-wrap-handler.test.ts b/packages/objectql/src/hook-binder-wrap-handler.test.ts new file mode 100644 index 00000000000..43109ed1c29 --- /dev/null +++ b/packages/objectql/src/hook-binder-wrap-handler.test.ts @@ -0,0 +1,104 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #24289 — `bindHooksToEngine`'s `wrapHandler` option: one wrap for every + * handler a hook runs in-process, applied after a string `handler` resolves. + * + * - an inline function, a name from the bind's `functions` and a name the + * same package registered on the engine all take the wrap; + * - a `body`'s handler (built by `bodyRunner`) never does; + * - control: a bind with no `wrapHandler` calls the resolved function with + * the engine's own context object, exactly as before. + */ + +import { describe, it, expect } from 'vitest'; +import { ObjectQL } from './engine.js'; +import { bindHooksToEngine } from './hook-binder.js'; +import type { Hook, HookContext } from '@objectstack/spec/data'; + +const ctx = (): HookContext => + ({ object: 'account', event: 'beforeInsert', input: { data: {} }, ql: undefined }) as HookContext; + +/** Marks each context the wrapped handler is called with, and records which hook it wrapped. */ +function marking(wrappedFor: string[]) { + return (handler: (context: HookContext) => unknown, hook: Hook) => { + wrappedFor.push(hook.name); + return (context: HookContext) => handler({ ...context, wrapped: hook.name } as HookContext); + }; +} + +const hook = (name: string, handler: Hook['handler']): Hook => ({ + name, + object: 'account', + events: ['beforeInsert'], + handler, +}); + +describe('#24289: bindHooksToEngine wrapHandler', () => { + it('wraps an inline handler and both kinds of named handler, once each, at bind', async () => { + const engine = new ObjectQL(); + engine.registerFunction('registered_fn', async (c: any) => { seen.push(`registered:${c.wrapped}`); }, { + packageId: 'app:wrap', + }); + const seen: string[] = []; + const wrappedFor: string[] = []; + + const result = bindHooksToEngine( + engine, + [ + hook('inline', async (c: any) => { seen.push(`inline:${c.wrapped}`); }), + hook('from_bind', 'bound_fn'), + hook('from_registry', 'registered_fn'), + ], + { + packageId: 'app:wrap', + functions: { bound_fn: async (c: any) => { seen.push(`bound:${c.wrapped}`); } }, + wrapHandler: marking(wrappedFor), + }, + ); + expect(result.errors).toEqual([]); + expect(wrappedFor).toEqual(['inline', 'from_bind', 'from_registry']); + + await engine.triggerHooks('beforeInsert', ctx()); + await engine.triggerHooks('beforeInsert', ctx()); + expect(seen.sort()).toEqual([ + 'bound:from_bind', 'bound:from_bind', + 'inline:inline', 'inline:inline', + 'registered:from_registry', 'registered:from_registry', + ]); + expect(wrappedFor, 'the wrap is applied at bind, never per event').toHaveLength(3); + }); + + it("never wraps the handler a body's runner builds", async () => { + const engine = new ObjectQL(); + const seen: unknown[] = []; + const wrappedFor: string[] = []; + bindHooksToEngine( + engine, + [{ ...hook('with_body', undefined), body: { language: 'js', source: 'return;' } } as Hook], + { + bodyRunner: () => async (c: any) => { seen.push(c.wrapped); }, + wrapHandler: marking(wrappedFor), + }, + ); + + await engine.triggerHooks('beforeInsert', ctx()); + expect(wrappedFor).toEqual([]); + expect(seen).toEqual([undefined]); + }); + + it("control: without wrapHandler, a named handler is called with the engine's own context", async () => { + const engine = new ObjectQL(); + const received: unknown[] = []; + const fn = async (c: unknown) => { received.push(c); }; + bindHooksToEngine(engine, [hook('plain_named', 'plain_fn'), hook('plain_inline', fn)], { + packageId: 'app:plain', + functions: { plain_fn: fn }, + }); + + const context = ctx(); + await engine.triggerHooks('beforeInsert', context); + expect(received).toEqual([context, context]); + expect(received.every((c) => c === context)).toBe(true); + }); +}); diff --git a/packages/objectql/src/hook-binder.ts b/packages/objectql/src/hook-binder.ts index 741c8aa00bf..39d0c639169 100644 --- a/packages/objectql/src/hook-binder.ts +++ b/packages/objectql/src/hook-binder.ts @@ -21,7 +21,7 @@ * metadata-aware behaviour lives in this binder + the wrapper module. */ -import type { Hook } from '@objectstack/spec/data'; +import type { Hook, HookContext } from '@objectstack/spec/data'; import { normalizeFlowFunctionEntry, type FlowFunctionEntry } from '@objectstack/spec/automation'; import type { ObjectQL, HookHandler } from './engine.js'; import { wrapDeclarativeHook, type HookDiagnosticsLogger } from './hook-wrappers.js'; @@ -58,6 +58,22 @@ export interface BindHooksOptions { */ bodyRunner?: (hook: Hook) => HookHandler | undefined; + /** + * Optional wrap for each handler the hook runs IN-PROCESS: an inline + * `handler` function, and the function a string `handler` names (from + * `functions` or the package's registered functions). It is applied here, + * after the name is resolved, so both kinds take the one wrap; the handler + * `bodyRunner` builds from a `body` never does. Called once per hook, at bind. + * + * The runtime passes the wrap that hands an in-process handler the + * `ctx.title(field?)` a sandboxed `body` gets (#24289). Absent, every handler + * binds exactly as resolved. + */ + wrapHandler?: ( + handler: (context: HookContext) => unknown, + hook: Hook, + ) => (context: HookContext) => unknown; + /** * When true, treat unresolved hooks (body present but no runner, or handler * string with no implementation) as fatal errors instead of warnings. Used @@ -392,6 +408,21 @@ function resolveHandler( return undefined; } + const fn = resolveInProcessHandler(engine, hook, opts); + if (!fn || typeof opts.wrapHandler !== 'function') return fn; + // The engine awaits a handler and discards its return value; so does this. + const wrapped = opts.wrapHandler(fn, hook); + return async (context) => { + await wrapped(context); + }; +} + +/** The in-process handler a hook with no `body` names: its inline function, or the function its string names. */ +function resolveInProcessHandler( + engine: ObjectQL, + hook: Hook, + opts: BindHooksOptions, +): HookHandler | undefined { const h = hook.handler; if (typeof h === 'function') return h as HookHandler; if (typeof h === 'string' && h.length > 0) { diff --git a/packages/runtime/src/app-artifact-handlers.ts b/packages/runtime/src/app-artifact-handlers.ts index f661ee94e9a..1b658c88ace 100644 --- a/packages/runtime/src/app-artifact-handlers.ts +++ b/packages/runtime/src/app-artifact-handlers.ts @@ -255,9 +255,6 @@ export function bindAppArtifactHandlers( ); } } - // [#24289] An inline `handler` function runs in-process; it is handed the - // `ctx.title(field?)` a `body` gets in the sandbox (see `hookWithInProcessTitle`). - hooks = hooks.map((hook) => hookWithInProcessTitle(hook, ql)); // Entries, not bare handlers: each function's declared `effect` // (#4396) rides along to the registry, where a `script` node reads // it to report what its run actually did. @@ -277,6 +274,11 @@ export function bindAppArtifactHandlers( packageId: owner, functions, bodyRunner: hookBodyRunnerFactory(new QuickJSScriptRunner(), { ql, logger, appId }), + // [#24289] A handler that runs in-process, an inline function or + // the function a string `handler` names, is handed the + // `ctx.title(field?)` a `body` gets in the sandbox. The binder + // applies it after resolving the name, so both take this one wrap. + wrapHandler: (handler, hook) => withInProcessHookTitle(handler, ql, String(hook.name)), }); out.hooks = hooks.length; out.functions = Object.keys(functions).length; @@ -339,25 +341,6 @@ export function bindAppArtifactHandlers( return out; } -/** - * [#24289] `hook` with its inline `handler` function handed `ctx.title(field?)` - * ({@link withInProcessHookTitle}, the one accessor a `body` is handed too), or - * `hook` itself when its `handler` is not a function. - * - * A copy, so the app's own declaration is never rewritten. Which of `body` and - * `handler` runs stays the binder's call: a hook carrying both binds its `body`, - * and the wrapped function is never consulted. - * - * A `handler` that NAMES a function (the deprecated string form, and what the - * build leaves for a callable it could not lower) also runs in-process, but the - * function is found by name inside the engine's binder, past this point, so - * it is not handed the accessor here. - */ -function hookWithInProcessTitle(hook: any, ql: IObjectQLEngine): any { - if (!hook || typeof hook.handler !== 'function') return hook; - return { ...hook, handler: withInProcessHookTitle(hook.handler, ql, String(hook.name)) }; -} - // ─── The job half (#21489) ───────────────────────────────────────────── /** diff --git a/packages/spec/src/contracts/objectql-engine.ts b/packages/spec/src/contracts/objectql-engine.ts index ad4b467a697..78adfe03b51 100644 --- a/packages/spec/src/contracts/objectql-engine.ts +++ b/packages/spec/src/contracts/objectql-engine.ts @@ -55,6 +55,7 @@ import type { IDataEngine } from './data-engine'; import type { IDataDriver } from './data-driver'; import type { FlowFunctionEffect, FlowFunctionEntry } from '../automation/flow-function.zod'; import type { ServiceObject } from '../data/object.zod'; +import type { Hook, HookContext } from '../data/hook.zod'; import type { BaseEngineOptions, EngineQueryOptions } from '../data/data-engine.zod'; /** @@ -403,6 +404,20 @@ export interface IObjectQLEngine extends IDataEngine { strict?: boolean; warnLegacyHandler?: boolean; metrics?: unknown; + /** + * Wraps each handler the hook runs IN-PROCESS: an inline `handler` + * function, and the function a string `handler` names (resolved from + * `functions` or the package's registered functions). Never applied to + * the handler `bodyRunner` builds from a `body`. Called once per hook, + * at bind; what it returns is what the engine calls. The engine awaits + * a handler and discards its return value. The runtime passes the + * wrap that hands an in-process handler `ctx.title(field?)` (#24289). + * Absent: every handler binds exactly as resolved. + */ + wrapHandler?: ( + handler: (context: HookContext) => unknown, + hook: Hook, + ) => (context: HookContext) => unknown; }, ): void; diff --git a/packages/verify/src/harness.named-hook-title-door.test.ts b/packages/verify/src/harness.named-hook-title-door.test.ts new file mode 100644 index 00000000000..3b076a13ac9 --- /dev/null +++ b/packages/verify/src/harness.named-hook-title-door.test.ts @@ -0,0 +1,125 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #24289 — a hook whose `handler` NAMES a function gets the same + * `ctx.title(field?)` on both doors of `bootStack`, as an inline handler and a + * `body` do (`harness.hook-title-door.test.ts`). + * + * The two doors name the function two ways, and both run it in-process: + * + * - the source door: `defineStack({ functions })` and a hook naming one entry; + * - the artifact door: a string `handler` and a `runtimeModule` that supplies + * the function, which is what `objectstack build` ships for a callable it + * could not lower to a `body`. + * + * Measured before (`origin/main` ee882b845): both doors refused every insert + * with `TypeError: ctx.title is not a function`. The binder resolved the name + * past the runtime's wrap of an inline handler, so the function was called + * with the engine's bare context. + */ + +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterAll, afterEach, beforeAll, describe, expect, it } from 'vitest'; +import { defineStack } from '@objectstack/spec'; +import { bootStack, type VerifyStack } from './harness.js'; + +const BOOT_TIMEOUT = 120_000; + +const manifest = { id: 'com.example.named-hook-title-door', name: 'Named Hook Title Door', namespace: 'nhd', version: '1.0.0', type: 'app' }; + +const product = { + name: 'nhd_product', label: 'Product', pluralLabel: 'Products', sharingModel: 'public_read_write', + nameField: 'label', + fields: { + name: { name: 'name', type: 'text', label: 'Name', required: true }, + code: { name: 'code', type: 'text', label: 'Code' }, + label: { name: 'label', type: 'formula', label: 'Label', expression: "record.name + ' (' + record.code + ')'" }, + }, +}; + +const line = { + name: 'nhd_line', label: 'Line', pluralLabel: 'Lines', sharingModel: 'public_read_write', + nameField: 'title', + fields: { + name: { name: 'name', type: 'text', label: 'Name', required: true }, + product: { name: 'product', type: 'lookup', label: 'Product', reference: 'nhd_product' }, + spare: { name: 'spare', type: 'lookup', label: 'Spare', reference: 'nhd_product' }, + title: { name: 'title', type: 'formula', label: 'Title', expression: "record.name + ' · ' + record.product.name" }, + own_title: { name: 'own_title', type: 'text', label: 'Own title' }, + product_title: { name: 'product_title', type: 'text', label: 'Product title' }, + spare_title: { name: 'spare_title', type: 'text', label: 'Spare title' }, + }, +}; + +/** One statement set: the named function's body on both doors. */ +const SOURCE = [ + 'ctx.input.own_title = String(await ctx.title());', + "ctx.input.product_title = String(await ctx.title('product'));", + "ctx.input.spare_title = String(await ctx.title('spare'));", +].join('\n'); + +const hook = { name: 'nhd_name_line', object: 'nhd_line', events: ['beforeInsert'], handler: 'nhd_stamp_titles' }; + +type TitleCtx = { input: Record; title: (field?: string) => Promise }; + +/** The configuration: the hook names an entry of the stack's own `functions`. */ +const app = (): Record => + defineStack({ + manifest, + objects: [product, line], + hooks: [hook], + functions: { + nhd_stamp_titles: async (ctx: TitleCtx) => { + ctx.input.own_title = String(await ctx.title()); + ctx.input.product_title = String(await ctx.title('product')); + ctx.input.spare_title = String(await ctx.title('spare')); + }, + }, + } as never) as unknown as Record; + +/** The artifact: the same hook, its function shipped in the runtime module beside it. */ +const artifact = (): Record => ({ + manifest, + objects: [product, line], + hooks: [hook], + runtimeModule: './runtime.mjs', +}); +const RUNTIME_MODULE = `export const functions = {\n nhd_stamp_titles: async (ctx) => {\n${SOURCE}\n },\n};\n`; + +/** What a `body` answers on a served runtime, the answer both doors owe. */ +const EXPECTED = { own: 'L1 · Widget', product: 'Widget (W-1)', spare: 'null' }; + +let hostRoot: string; +beforeAll(() => { + hostRoot = mkdtempSync(join(tmpdir(), 'verify-named-hook-title-door-')); + mkdirSync(join(hostRoot, 'dist')); + writeFileSync(join(hostRoot, 'dist', 'objectstack.json'), JSON.stringify(artifact())); + writeFileSync(join(hostRoot, 'dist', 'runtime.mjs'), RUNTIME_MODULE); +}); +afterAll(() => { + if (hostRoot) rmSync(hostRoot, { recursive: true, force: true }); +}); + +const live: VerifyStack[] = []; +afterEach(async () => { + for (const stack of live.splice(0)) await stack.stop(); +}); + +const titlesOn = async (stack: VerifyStack): Promise> => { + live.push(stack); + const widget = await stack.hooks.run('nhd_product', 'insert', { name: 'Widget', code: 'W-1' }, { system: true }); + const row = await stack.hooks.run('nhd_line', 'insert', { name: 'L1', product: widget.id }, { system: true }); + return { own: row.own_title, product: row.product_title, spare: row.spare_title }; +}; + +describe('#24289: a hook whose handler names a function gets ctx.title on both bootStack doors', () => { + it('the artifact door: a string handler resolved from the runtime module', async () => { + expect(await titlesOn(await bootStack(app(), { hostRoot, artifact: 'dist/objectstack.json' }))).toEqual(EXPECTED); + }, BOOT_TIMEOUT); + + it("the source door: a string handler resolved from the stack's functions", async () => { + expect(await titlesOn(await bootStack(app(), { hostRoot }))).toEqual(EXPECTED); + }, BOOT_TIMEOUT); +});