fix(metadata-protocol)!: the save door refuses a hook that names a function in handler and carries no body (#21658)

.changeset/21658-hook-handler-without-body-save-door.md

@@ -0,0 +1,26 @@
+---
+'@objectstack/metadata-protocol': minor
+---
+
+The runtime save door refuses a hook whose `handler` names a function and that carries no `body`: a hook stored there ships with no code package, so that name can never bind
+
+Clause-②: no (narrowing)
+
+<!-- adr-0087: not-required (no-migration-prescription) A validity narrowing at one runtime write door over an existing key: no key of `HookSchema` or of any other metadata schema is removed, renamed or re-shaped, so there is no tombstone and nothing mechanical for `objectstack migrate meta` to rewrite. What `body` a refused hook should carry is authoring intent no conversion entry can decide. New saves are refused with the remedy; a row stored before this change keeps its bytes, and no stored row is re-saved (measured: `migrateStoredMetadata` records such a row canonical and writes nothing). The census of writers through this door, taken first: Studio at the objectui pin (`ab18797215`) creates a hook from a skeleton that carries a `body` and re-saves the body it lists (`ObjectHooksPanel`); objectstack `examples/**` and `packages/qa/**` declare no hook with a string `handler` (the only string `handler` there is a job's) and seed no `sys_metadata` hook rows; the platform checklist saves no such hook; the artifact, boot and install-local doors never call `saveMetaItem` for a hook (every call site in the tree saves a fixed type other than `hook`, or forwards an author's request: the REST and dispatcher `/meta` saves, `migrateStoredMetadata`, `duplicatePackage`). Package duplication of a package holding such a row now reports that row as failed with this refusal (measured). Hosted tenants and the cloud AI author were not measured. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id covers this rule and this diff adds none (not registered / already-registered); and the change narrows what a runtime write door accepts, not a runtime interface or a type surface alone (not runtime-interface-only / type-surface-only). -->
+
+**BREAKING** accept-set narrowing at the runtime save door, shipped as `minor` under the repo's launch-window convention for breaking changes, the grade the same door's earlier refusals shipped with.
+
+**One rule.** `saveMetaItem`, which `PUT /api/v1/meta/hook/:name` and the dispatcher's metadata save both call, now refuses a `hook` whose `handler` is a function name and that carries no `body`. A hook stored through this door ships with no code package, so it holds no functions, and a `handler` name resolves only inside the hook's own package. Before this change the door answered 200, the runtime then refused the hook at bind (`INVALID_REFERENCE` / 400, in the server log only), and the hook never ran. The refusal is `VALIDATION_ERROR` / 400, in draft and in publish mode, before anything is stored or bound. It names the hook and the function, and prescribes a `body`.
+
+**Before and after** (with `{ name: 'stamp_status', object: 'crm_note', events: ['beforeInsert'], handler: 'x_stamp' }`):
+
+- Before: 200 `Saved hook 'stamp_status'`, the row stored, the hook refused at bind and never run, and nothing on the response said so.
+- After: 400 `VALIDATION_ERROR`, naming `stamp_status` and `x_stamp`, and nothing stored.
+
+**What still saves.** A hook with a `body`. A hook carrying both a `body` and a `handler`: the binder runs the body and never consults the name, and the install-local door accepts the same shape. A malformed `body` still gets the type schema's located `422 INVALID_METADATA`.
+
+**What is unchanged.** `HookSchema` still accepts the string `handler`, because a build artifact carries it: `objectstack build` lowers an inline function to the hook's name and ships the function in the artifact's runtime module. A hook in an artifact or a `defineStack` config binds to its own package's functions on its own door, which never reaches this one. `os validate` and `os build` are unchanged.
+
+**Rows stored before this change.** They keep their bytes, nothing re-saves them, and the runtime refuses them at bind as before. A new save of one, a re-save included, is refused until it carries a `body`. Package duplication reports such a row as failed with this refusal; `migrate meta --stored` leaves it as it is. Delete stays open.
+
+**The fix.** Give the hook a `body`: sandboxed JS (`{ language: 'js', source }`) or an expression (`{ language: 'expression', source }`). A hook that must run a package's own function belongs in that package's code, where its `handler` resolves.

packages/metadata-protocol/src/protocol.invalid-metadata-422-face-inventory.test.ts

@@ -492,3 +492,81 @@ describe('[#21565] a hook body bound to a stored-metadata table is refused at th
         expect([...rows.values()].map((r) => [r.type, r.name])).toEqual([['hook', 'stamp_status']]);
     });
 });
+
+// ═══════════════════════════════════════════════════════════════════════════
+// 7. #21658 — the `hook` door refuses a `handler` name with no `body`
+// ═══════════════════════════════════════════════════════════════════════════
+//
+// The same door as section 6, and the contrast to it: this refusal is NOT the
+// type schema's. `HookSchema` keeps accepting the string `handler`, because a
+// build artifact legitimately carries that form, and the artifact door never
+// reaches `saveMetaItem`. What this door stores ships with no code package,
+// and a `handler` name resolves only inside the hook's own package, so a hook
+// naming a function and carrying no `body` can never bind once stored. The
+// door refuses it with `VALIDATION_ERROR` / 400 — the envelope of the name
+// check every body passes — before anything is stored, in publish and in draft
+// mode. Rides this file's pinned engine double, as section 6 does.
+
+describe('[#21658] a hook naming a function in `handler` with no `body` is refused at the metadata door', () => {
+    const handlerOnly = () => ({
+        name: 'stamp_status',
+        object: 'hks_note',
+        events: ['beforeInsert'],
+        handler: 'x_stamp',
+    });
+    const body = { language: 'js', source: "ctx.input.status = 'seen';" };
+
+    it.each([
+        ['publish', undefined],
+        ['draft', 'draft'],
+    ] as const)('%s mode — VALIDATION_ERROR / 400, naming the hook and its handler, nothing stored', async (_label, mode) => {
+        const { protocol, rows } = makeProtocol();
+        let err: any;
+        try {
+            await protocol.saveMetaItem({
+                type: 'hook',
+                name: 'stamp_status',
+                item: handlerOnly(),
+                writeFace: 'meta-envelope',
+                actor: 'usr_admin',
+                ...(mode ? { mode } : {}),
+            });
+        } catch (e) {
+            err = e;
+        }
+
+        expect(err).toBeInstanceOf(Error);
+        expect({ code: err.code, status: err.status }).toEqual({ code: 'VALIDATION_ERROR', status: 400 });
+        expect(err.message).toContain("'stamp_status'");
+        expect(err.message).toContain("'x_stamp'");
+        expect(err.message).toContain('Give it a `body`');
+        expect(rows.size).toBe(0);
+    });
+
+    it('CONTROL — the same hook with a `body` saves', async () => {
+        const { protocol, rows } = makeProtocol();
+        const { handler: _dropped, ...withoutHandler } = handlerOnly();
+        const result = await saveHookAsAdministrator(protocol, { ...withoutHandler, body });
+
+        expect(result instanceof Error ? `${result.message} ${JSON.stringify((result as any).issues ?? [])}` : 'stored').toBe('stored');
+        expect([...rows.values()].map((r) => [r.type, r.name])).toEqual([['hook', 'stamp_status']]);
+    });
+
+    it('a `body` beside the `handler` saves: the binder runs the body and never consults the name', async () => {
+        const { protocol, rows } = makeProtocol();
+        const result = await saveHookAsAdministrator(protocol, { ...handlerOnly(), body });
+
+        expect(result instanceof Error ? `${result.message} ${JSON.stringify((result as any).issues ?? [])}` : 'stored').toBe('stored');
+        expect([...rows.values()].map((r) => [r.type, r.name])).toEqual([['hook', 'stamp_status']]);
+    });
+
+    it('a malformed `body` beside the `handler` gets the schema\'s located 422, not "give it a body"', async () => {
+        const { protocol, rows } = makeProtocol();
+        const err = await saveHookAsAdministrator(protocol, { ...handlerOnly(), body: 'return;' });
+
+        expect(err).toBeInstanceOf(Error);
+        expect({ code: err.code, status: err.status }).toEqual({ code: 'INVALID_METADATA', status: 422 });
+        expect((err.issues as Array<{ path?: string }>).map((i) => i.path)).toContain('body');
+        expect(rows.size).toBe(0);
+    });
+});

packages/metadata-protocol/src/protocol.ts

@@ -779,6 +779,70 @@ function resolveOverlaySchema(type: string, _item: unknown): z.ZodTypeAny | null
     return getMetadataTypeSchema(singular) ?? null;
 }
 
+/**
+ * [#21658] The save door's refusal of a `hook` whose `handler` names a
+ * function and that carries no `body`: such a hook can never run once this
+ * door has stored it.
+ *
+ * Why it can never bind. A hook's `handler` name resolves inside the hook's
+ * own package only (the maintainer's ruling on #21604, letter B; the binder's
+ * `resolveHandler` in `@objectstack/objectql`'s `hook-binder.ts`). A hook this
+ * door stores ships with no code package: the runtime binds every stored hook
+ * under the synthetic owner `metadata-service` (`ObjectQLPlugin`'s authored
+ * hook re-sync), with no `functions` map, and no package of that name
+ * registers functions. So the name has nothing to resolve against, and the
+ * binder refuses the hook at registration (`INVALID_REFERENCE` / 400, logged
+ * at `error`) after this door has already answered success. Refusing it here
+ * says so to the author, before anything is stored.
+ *
+ * The predicate is the binder's own body-first test: a `body` object is bound
+ * through the body runner and the `handler` is never consulted, so a hook
+ * carrying BOTH a `body` and a `handler` saves (its body runs), as it installs
+ * on the install-local door. Asked after the type schema has accepted the
+ * body, so `body` here is either absent or a declared hook body, and a
+ * malformed `body` gets the schema's own located `422` instead of this
+ * refusal's "give it a body".
+ *
+ * ⛔ Not a `HookSchema` rule: a build artifact legitimately carries the string
+ * form (`objectstack build` lowers an inline function to the hook's name and
+ * ships the function in the artifact's runtime module), and the artifact and
+ * boot doors never reach `saveMetaItem`. This is the runtime-authoring door's
+ * rule only, the same shape install-local refuses on its own door (#21585).
+ *
+ * Every writer through this door is judged: the REST and dispatcher saves, in
+ * draft and in publish mode, and the two server-stated re-savers
+ * (`migrateStoredMetadata`, `duplicatePackage`), which record this refusal as
+ * the row's failure. A row stored before this rule keeps its bytes.
+ *
+ * `VALIDATION_ERROR` / 400, the envelope of the name check the door runs on
+ * every body (`savedItemNameRefusal`). The message names the hook and its
+ * `handler`, prescribes the `body` first, and only then explains: a 4xx
+ * message crosses the REST boundary bounded at 500 characters with its TAIL
+ * truncated, and the whole sentence stays under that bound for any hook and
+ * function name shorter than about 65 characters each. Runtime words carry no
+ * tracker number.
+ */
+function runtimeHookWithoutBodyRefusal(
+    singularType: string,
+    item: unknown,
+    saveName: string,
+): (Error & { code: 'VALIDATION_ERROR'; status: 400 }) | undefined {
+    if (singularType !== 'hook') return undefined;
+    if (!item || typeof item !== 'object' || Array.isArray(item)) return undefined;
+    const hook = item as { handler?: unknown; body?: unknown };
+    if (hook.body && typeof hook.body === 'object') return undefined;
+    if (typeof hook.handler !== 'string' || hook.handler === '') return undefined;
+    const err = new Error(
+        `Invalid hook: '${saveName}' names the function '${hook.handler}' in its \`handler\` and carries no \`body\`, `
+        + 'so it can never run. Give it a `body` (sandboxed JS, `{ language: \'js\', source }`, or an expression), '
+        + 'which is stored with the hook. A hook saved through the metadata API ships with no code package, so it '
+        + "holds no functions, and a `handler` name resolves only inside the hook's own package.",
+    ) as Error & { code: 'VALIDATION_ERROR'; status: 400 };
+    err.code = 'VALIDATION_ERROR';
+    err.status = 400;
+    return err;
+}
+
 /**
  * One entry of the `422 INVALID_METADATA` envelope's `issues[]` — the shape
  * Studio's designer keys on to highlight the offending form control.
@@ -18815,6 +18879,17 @@ export class ObjectStackProtocolImplementation implements
             }
         }
 
+        // [#21658] A hook whose `handler` names a function and that carries
+        // no `body` can never run once stored here: a stored hook ships with
+        // no code package, and a `handler` name resolves only inside the
+        // hook's own package. Refused in draft and in publish mode, after the
+        // schema (so `body` is absent or a declared body) and before the
+        // authoring gate and every write. See {@link runtimeHookWithoutBodyRefusal}.
+        {
+            const hookRefusal = runtimeHookWithoutBodyRefusal(singularType, request.item, request.name);
+            if (hookRefusal) throw hookRefusal;
+        }
+
         // The #4463 runtime authoring gate — the shared author-time rule
         // registry, on the write path. `active` saves only (D1): this is the
         // publish verb, and it is the same table `os build` gates on. Placed