refactor(core): share the two hook-dispatch flavours + gate the paired kernel pins (#5282)

.changeset/kernel-hook-dispatch-shared.md

@@ -0,0 +1,58 @@
+---
+"@objectstack/core": patch
+---
+
+refactor(core): one implementation per hook-dispatch flavour, plus a paired-pin gate (#5282)
+
+`ObjectKernel` does not extend `ObjectKernelBase` — it is a standalone
+production kernel with its own `hooks` map, and only `LiteKernel` extends the
+base. Lifecycle-hook dispatch therefore existed **twice**, with no shared code
+path: the base's `triggerHook` (isolating) / `triggerHookOrThrow` (propagating) /
+`context.trigger` on one side, and `ObjectKernel`'s private
+`triggerShutdownHookIsolating` / `context.trigger` on the other. The two
+isolating loops printed the same `Hook handler failed: kernel:shutdown` line
+because someone typed it twice.
+
+That seam produced three consecutive bugs, each the same shape — one hook name
+meaning opposite things on the two kernels: `kernel:ready` (#5170),
+`kernel:bootstrapped` / `kernel:listening` (#5257, where a swallowed
+`server.listen()` failure let a process print "✅ Bootstrap complete" with
+nothing listening), and `kernel:shutdown` in the other direction (#5274, where
+one bad handler skipped every `destroy()`).
+
+**No behaviour change.** The two dispatch flavours move verbatim into an
+internal module, `packages/core/src/hook-dispatch.ts`, which both kernels now
+call:
+
+- `dispatchHookIsolating` — a failing handler is logged as
+  `Hook handler failed: <name>` and the remaining handlers still run.
+- `dispatchHookPropagating` — the first failure escapes unwrapped and the
+  handlers behind it are skipped.
+
+Every call path keeps the flavour, the log wording and the trace line it had
+before, including the one asymmetry inside the propagating flavour:
+`PluginContext.trigger` has never emitted the `Triggering hook: <name>` trace on
+either kernel, so it still does not. The kernels' two `hooks` maps are
+deliberately **not** unified, and `ObjectKernel` deliberately does **not** gain a
+base class — both were considered and ruled out of scope.
+
+How "no behaviour change" was proved: the paired kernel pins from #5170 / #5257 /
+#5274 pass untouched, and deleting the shared dispatcher's error log now turns
+**both** kernels' test files red from a single edit — a property the hand-mirrored
+copies could not have (editing `ObjectKernel`'s private loop could never turn
+`lite-kernel.test.ts` red).
+
+Shared dispatch cannot cover the residual two-maps seam, so the pairing of the
+tests is now a gate rather than a convention: `pnpm check:kernel-hook-pairs`
+(`scripts/check-kernel-hook-pairs.mjs`, wired into the ESLint job) requires every
+`kernel:*` hook dispatched in `packages/core/src` to be named in a test title in
+**both** `kernel.test.ts` and `lite-kernel.test.ts`, and fails naming the hook
+and the side that lacks it. A fifth lifecycle hook can no longer arrive paired on
+one kernel only.
+
+Also pinned, deliberately unchanged: `kernel:shutdown` has two dispatch paths
+with different flavours on both kernels — the kernel's own teardown isolates,
+while a plugin calling `ctx.trigger('kernel:shutdown')` by hand propagates.
+Nothing in the repo triggers it by hand today, so this is dormant; it is now a
+documented fact with a named test on each side rather than a surprise found at
+teardown.

.github/workflows/lint.yml

@@ -448,6 +448,32 @@ jobs:
       - name: Engine test-double contract gate
         run: pnpm check:engine-double-contract
 
+      # Paired kernel-hook pin gate (#5282, from #5170 / #5257 / #5274). The two
+      # kernels — ObjectKernel (production) and LiteKernel (vitest / serverless /
+      # edge) — run the same plugin code and the same hook vocabulary, but do NOT
+      # share a class: ObjectKernel does not extend ObjectKernelBase, so each
+      # keeps its own hooks map. #5282 shared the two dispatch LOOPS
+      # (packages/core/src/hook-dispatch.ts); this gate covers what sharing them
+      # cannot: three consecutive bugs were all "one hook name means opposite
+      # things on the two kernels" (kernel:ready swallowed on one side and fatal
+      # on the other; kernel:listening swallowing a failed server.listen() behind
+      # a cheerful "Bootstrap complete"; kernel:shutdown skipping every destroy()
+      # in the other direction), and each was caught by a human noticing the
+      # asymmetry. What held the kernels together afterwards was a pair of
+      # hand-written tests — a convention, not a mechanism: a fifth lifecycle
+      # hook gets no pairing automatically and nothing goes red when it is
+      # missing. So every kernel:* hook DISPATCHED in packages/core/src must be
+      # named in a test title in BOTH kernel.test.ts and lite-kernel.test.ts, and
+      # a missing pair fails naming the hook and the side that lacks it.
+      # Subscriptions (ctx.hook) are deliberately not dispatches. Static AST over
+      # five files, no build needed, so it belongs in this job. Runs its own
+      # --self-test first: the detector can be broken while every hook is fine,
+      # and a scan that stops matching would report OK while reading nothing
+      # (#4868's family). Measured against main's corpus before being pinned
+      # here: 4 dispatched hooks, 0 problems.
+      - name: Paired kernel-hook pin gate
+        run: pnpm check:kernel-hook-pairs
+
       # Resume-authority declaration gate (#5561, from #3823). The #3801 resume
       # gate keys on the SUSPENDED NODE, so it covers a pausing node type exactly
       # when that type's author remembered to declare `resumeAuthority`. #3823 is

package.json

@@ -52,6 +52,7 @@
     "check:wildcard-fallthrough": "node scripts/check-wildcard-fallthrough.mjs --self-test && node scripts/check-wildcard-fallthrough.mjs",
     "check:meta-type-normalized": "node scripts/check-meta-type-normalized.mjs --self-test && node scripts/check-meta-type-normalized.mjs",
     "check:init-service-contract": "node scripts/check-init-service-contract.mjs --self-test && node scripts/check-init-service-contract.mjs",
+    "check:kernel-hook-pairs": "node scripts/check-kernel-hook-pairs.mjs --self-test && node scripts/check-kernel-hook-pairs.mjs",
     "check:durability-log-level": "node scripts/check-durability-degradation-log-level.mjs --self-test && node scripts/check-durability-degradation-log-level.mjs",
     "check:startup-registry-verdict": "node scripts/check-startup-registry-verdict.mjs --self-test && node scripts/check-startup-registry-verdict.mjs",
     "check:console-sha": "node scripts/check-console-sha.mjs",

packages/core/src/hook-dispatch.test.ts

@@ -0,0 +1,183 @@
+// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
+
+import { describe, it, expect, vi } from 'vitest';
+import {
+    dispatchHookIsolating,
+    dispatchHookPropagating,
+    type HookDispatchLogger,
+    type HookHandler,
+} from './hook-dispatch.js';
+
+/**
+ * Direct pins for the two shared dispatch flavours (#5282).
+ *
+ * These exist as well as — never instead of — the paired kernel pins in
+ * `kernel.test.ts` / `lite-kernel.test.ts`. The kernel pins say what each hook
+ * MEANS on each kernel; these say what each flavour DOES, once, at the single
+ * place both kernels now reach. The division matters for the reverse
+ * verification: mutating a function here turns both kernels' files red in one
+ * edit, which is the property option B was ruled to buy.
+ */
+
+const makeLogger = () => {
+    const debug = vi.fn();
+    const error = vi.fn();
+    const logger: HookDispatchLogger = { debug, error };
+    return { logger, debug, error };
+};
+
+describe('hook dispatch (shared by ObjectKernel and ObjectKernelBase, #5282)', () => {
+    describe('dispatchHookIsolating', () => {
+        it('runs every remaining handler when one throws, in registration order', async () => {
+            const reached: string[] = [];
+            const { logger } = makeLogger();
+            const handlers: HookHandler[] = [
+                async () => { reached.push('first'); },
+                async () => { throw new Error('boom'); },
+                async () => { reached.push('third'); },
+            ];
+
+            await expect(dispatchHookIsolating('kernel:shutdown', handlers, logger)).resolves.toBeUndefined();
+
+            expect(reached).toEqual(['first', 'third']);
+        });
+
+        it('logs each failure as `Hook handler failed: <name>` with the original error', async () => {
+            const { logger, error } = makeLogger();
+            const boom = new Error('boom');
+
+            await dispatchHookIsolating('kernel:shutdown', [async () => { throw boom; }], logger);
+
+            // The wording is contract: both kernels' pins assert this exact
+            // line, and before #5282 it was typed out twice to keep them equal.
+            expect(error).toHaveBeenCalledWith('Hook handler failed: kernel:shutdown', boom);
+        });
+
+        it('traces `Triggering hook: <name>` with the handler count before running any handler', async () => {
+            const { logger, debug } = makeLogger();
+            const order: string[] = [];
+            debug.mockImplementation(() => { order.push('trace'); });
+
+            await dispatchHookIsolating(
+                'kernel:shutdown',
+                [async () => { order.push('handler'); }, async () => { order.push('handler'); }],
+                logger,
+            );
+
+            expect(debug).toHaveBeenCalledWith('Triggering hook: kernel:shutdown', {
+                hook: 'kernel:shutdown',
+                handlerCount: 2,
+            });
+            expect(order).toEqual(['trace', 'handler', 'handler']);
+        });
+
+        it('passes every argument through to each handler', async () => {
+            const { logger } = makeLogger();
+            const seen: unknown[][] = [];
+
+            await dispatchHookIsolating(
+                'data:beforeInsert',
+                [(...args: unknown[]) => { seen.push(args); }, (...args: unknown[]) => { seen.push(args); }],
+                logger,
+                ['sys_user', { id: 'u1' }],
+            );
+
+            expect(seen).toEqual([
+                ['sys_user', { id: 'u1' }],
+                ['sys_user', { id: 'u1' }],
+            ]);
+        });
+
+        it('takes the handler array by reference — a handler subscribing mid-dispatch still runs', async () => {
+            // Both hand-written originals read the live array off the hooks map
+            // and iterated it directly. Pinned so a defensive `[...handlers]`
+            // copy cannot change dispatch semantics as a tidy-up.
+            const { logger } = makeLogger();
+            const reached: string[] = [];
+            const handlers: HookHandler[] = [];
+            handlers.push(async () => {
+                reached.push('first');
+                handlers.push(async () => { reached.push('late-subscriber'); });
+            });
+
+            await dispatchHookIsolating('kernel:shutdown', handlers, logger);
+
+            expect(reached).toEqual(['first', 'late-subscriber']);
+        });
+    });
+
+    describe('dispatchHookPropagating', () => {
+        it('lets the first failure escape unwrapped and skips the handlers behind it', async () => {
+            const reached: string[] = [];
+            const { logger, error } = makeLogger();
+            const boom = new Error('listen EACCES: permission denied 0.0.0.0:80');
+            const handlers: HookHandler[] = [
+                async () => { reached.push('first'); },
+                async () => { throw boom; },
+                async () => { reached.push('never'); },
+            ];
+
+            // Identity, not just message: the boot path re-throws the original
+            // error to its caller, it does not wrap or re-create it.
+            await expect(dispatchHookPropagating('kernel:listening', handlers, logger)).rejects.toBe(boom);
+
+            expect(reached).toEqual(['first']);
+            // A propagated failure is the caller's to report — the dispatcher
+            // does not also log it.
+            expect(error).not.toHaveBeenCalled();
+        });
+
+        it('traces `Triggering hook: <name>` with the handler count when a logger is supplied', async () => {
+            const { logger, debug } = makeLogger();
+
+            await dispatchHookPropagating('kernel:ready', [async () => {}], logger);
+
+            expect(debug).toHaveBeenCalledWith('Triggering hook: kernel:ready', {
+                hook: 'kernel:ready',
+                handlerCount: 1,
+            });
+        });
+
+        it('emits NO trace when no logger is supplied — the `context.trigger` shape', async () => {
+            // `PluginContext.trigger` has never logged a dispatch trace on
+            // either kernel. Handing it a logger would add a debug line to a
+            // path that has never had one; #5282 preserves every call path's
+            // semantics exactly, so the absence is pinned rather than assumed.
+            const { logger, debug } = makeLogger();
+            const reached: string[] = [];
+
+            await dispatchHookPropagating('kernel:ready', [async () => { reached.push('ran'); }], undefined);
+
+            expect(reached).toEqual(['ran']);
+            expect(debug).not.toHaveBeenCalled();
+            expect(logger.debug).not.toHaveBeenCalled();
+        });
+
+        it('passes every argument through to each handler', async () => {
+            const seen: unknown[][] = [];
+
+            await dispatchHookPropagating(
+                'metadata:reloaded',
+                [(...args: unknown[]) => { seen.push(args); }],
+                undefined,
+                ['sys_user'],
+            );
+
+            expect(seen).toEqual([['sys_user']]);
+        });
+    });
+
+    it('dispatches nothing, and never throws, for a hook with no subscribers', async () => {
+        const { logger, debug, error } = makeLogger();
+
+        await expect(dispatchHookIsolating('kernel:shutdown', [], logger)).resolves.toBeUndefined();
+        await expect(dispatchHookPropagating('kernel:ready', [], logger)).resolves.toBeUndefined();
+
+        expect(error).not.toHaveBeenCalled();
+        expect(debug).toHaveBeenCalledTimes(2);
+        expect(debug.mock.calls.map((c) => c[1])).toEqual([
+            { hook: 'kernel:shutdown', handlerCount: 0 },
+            { hook: 'kernel:ready', handlerCount: 0 },
+        ]);
+    });
+});