Skip to content

Commit d6d1a50

Browse files
os-zhuangclaude
andauthored
refactor(core): share the two hook-dispatch flavours + gate the paired kernel pins (#5282) (#6672)
* refactor(core): share the two hook-dispatch flavours + gate the paired kernel pins (#5282) `ObjectKernel` does not extend `ObjectKernelBase`, so lifecycle-hook dispatch existed twice with no shared code path — the isolating loops printed the same `Hook handler failed: <name>` line only because it was typed twice. Three consecutive bugs grew on that seam (#5170 / #5257 / #5274), each "one hook name means opposite things on the two kernels". B half: the two flavours move verbatim into `packages/core/src/hook-dispatch.ts` (`dispatchHookIsolating` / `dispatchHookPropagating`), called by both kernels. Every call path keeps its flavour, its log wording and its trace line — including `PluginContext.trigger`, which has never emitted a trace and still does not. The two `hooks` maps stay separate and `ObjectKernel` gains no base class (both out of scope by the ruling). C half: `scripts/check-kernel-hook-pairs.mjs` 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`, failing with the hook and the missing side. Measured against main's corpus first: 4 hooks, 0 problems. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MwoubC3jL271FYt9rGXwxb * fixup: no new type-check debt (explicit .js import + ctx annotations) --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent bf972e7 commit d6d1a50

10 files changed

Lines changed: 1047 additions & 52 deletions

File tree

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

‎.github/workflows/lint.yml‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -448,6 +448,32 @@ jobs:
448448
- name: Engine test-double contract gate
449449
run: pnpm check:engine-double-contract
450450

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

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
"check:wildcard-fallthrough": "node scripts/check-wildcard-fallthrough.mjs --self-test && node scripts/check-wildcard-fallthrough.mjs",
5454
"check:meta-type-normalized": "node scripts/check-meta-type-normalized.mjs --self-test && node scripts/check-meta-type-normalized.mjs",
5555
"check:init-service-contract": "node scripts/check-init-service-contract.mjs --self-test && node scripts/check-init-service-contract.mjs",
56+
"check:kernel-hook-pairs": "node scripts/check-kernel-hook-pairs.mjs --self-test && node scripts/check-kernel-hook-pairs.mjs",
5657
"check:durability-log-level": "node scripts/check-durability-degradation-log-level.mjs --self-test && node scripts/check-durability-degradation-log-level.mjs",
5758
"check:startup-registry-verdict": "node scripts/check-startup-registry-verdict.mjs --self-test && node scripts/check-startup-registry-verdict.mjs",
5859
"check:console-sha": "node scripts/check-console-sha.mjs",
Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect, vi } from 'vitest';
4+
import {
5+
dispatchHookIsolating,
6+
dispatchHookPropagating,
7+
type HookDispatchLogger,
8+
type HookHandler,
9+
} from './hook-dispatch.js';
10+
11+
/**
12+
* Direct pins for the two shared dispatch flavours (#5282).
13+
*
14+
* These exist as well as — never instead of — the paired kernel pins in
15+
* `kernel.test.ts` / `lite-kernel.test.ts`. The kernel pins say what each hook
16+
* MEANS on each kernel; these say what each flavour DOES, once, at the single
17+
* place both kernels now reach. The division matters for the reverse
18+
* verification: mutating a function here turns both kernels' files red in one
19+
* edit, which is the property option B was ruled to buy.
20+
*/
21+
22+
const makeLogger = () => {
23+
const debug = vi.fn();
24+
const error = vi.fn();
25+
const logger: HookDispatchLogger = { debug, error };
26+
return { logger, debug, error };
27+
};
28+
29+
describe('hook dispatch (shared by ObjectKernel and ObjectKernelBase, #5282)', () => {
30+
describe('dispatchHookIsolating', () => {
31+
it('runs every remaining handler when one throws, in registration order', async () => {
32+
const reached: string[] = [];
33+
const { logger } = makeLogger();
34+
const handlers: HookHandler[] = [
35+
async () => { reached.push('first'); },
36+
async () => { throw new Error('boom'); },
37+
async () => { reached.push('third'); },
38+
];
39+
40+
await expect(dispatchHookIsolating('kernel:shutdown', handlers, logger)).resolves.toBeUndefined();
41+
42+
expect(reached).toEqual(['first', 'third']);
43+
});
44+
45+
it('logs each failure as `Hook handler failed: <name>` with the original error', async () => {
46+
const { logger, error } = makeLogger();
47+
const boom = new Error('boom');
48+
49+
await dispatchHookIsolating('kernel:shutdown', [async () => { throw boom; }], logger);
50+
51+
// The wording is contract: both kernels' pins assert this exact
52+
// line, and before #5282 it was typed out twice to keep them equal.
53+
expect(error).toHaveBeenCalledWith('Hook handler failed: kernel:shutdown', boom);
54+
});
55+
56+
it('traces `Triggering hook: <name>` with the handler count before running any handler', async () => {
57+
const { logger, debug } = makeLogger();
58+
const order: string[] = [];
59+
debug.mockImplementation(() => { order.push('trace'); });
60+
61+
await dispatchHookIsolating(
62+
'kernel:shutdown',
63+
[async () => { order.push('handler'); }, async () => { order.push('handler'); }],
64+
logger,
65+
);
66+
67+
expect(debug).toHaveBeenCalledWith('Triggering hook: kernel:shutdown', {
68+
hook: 'kernel:shutdown',
69+
handlerCount: 2,
70+
});
71+
expect(order).toEqual(['trace', 'handler', 'handler']);
72+
});
73+
74+
it('passes every argument through to each handler', async () => {
75+
const { logger } = makeLogger();
76+
const seen: unknown[][] = [];
77+
78+
await dispatchHookIsolating(
79+
'data:beforeInsert',
80+
[(...args: unknown[]) => { seen.push(args); }, (...args: unknown[]) => { seen.push(args); }],
81+
logger,
82+
['sys_user', { id: 'u1' }],
83+
);
84+
85+
expect(seen).toEqual([
86+
['sys_user', { id: 'u1' }],
87+
['sys_user', { id: 'u1' }],
88+
]);
89+
});
90+
91+
it('takes the handler array by reference — a handler subscribing mid-dispatch still runs', async () => {
92+
// Both hand-written originals read the live array off the hooks map
93+
// and iterated it directly. Pinned so a defensive `[...handlers]`
94+
// copy cannot change dispatch semantics as a tidy-up.
95+
const { logger } = makeLogger();
96+
const reached: string[] = [];
97+
const handlers: HookHandler[] = [];
98+
handlers.push(async () => {
99+
reached.push('first');
100+
handlers.push(async () => { reached.push('late-subscriber'); });
101+
});
102+
103+
await dispatchHookIsolating('kernel:shutdown', handlers, logger);
104+
105+
expect(reached).toEqual(['first', 'late-subscriber']);
106+
});
107+
});
108+
109+
describe('dispatchHookPropagating', () => {
110+
it('lets the first failure escape unwrapped and skips the handlers behind it', async () => {
111+
const reached: string[] = [];
112+
const { logger, error } = makeLogger();
113+
const boom = new Error('listen EACCES: permission denied 0.0.0.0:80');
114+
const handlers: HookHandler[] = [
115+
async () => { reached.push('first'); },
116+
async () => { throw boom; },
117+
async () => { reached.push('never'); },
118+
];
119+
120+
// Identity, not just message: the boot path re-throws the original
121+
// error to its caller, it does not wrap or re-create it.
122+
await expect(dispatchHookPropagating('kernel:listening', handlers, logger)).rejects.toBe(boom);
123+
124+
expect(reached).toEqual(['first']);
125+
// A propagated failure is the caller's to report — the dispatcher
126+
// does not also log it.
127+
expect(error).not.toHaveBeenCalled();
128+
});
129+
130+
it('traces `Triggering hook: <name>` with the handler count when a logger is supplied', async () => {
131+
const { logger, debug } = makeLogger();
132+
133+
await dispatchHookPropagating('kernel:ready', [async () => {}], logger);
134+
135+
expect(debug).toHaveBeenCalledWith('Triggering hook: kernel:ready', {
136+
hook: 'kernel:ready',
137+
handlerCount: 1,
138+
});
139+
});
140+
141+
it('emits NO trace when no logger is supplied — the `context.trigger` shape', async () => {
142+
// `PluginContext.trigger` has never logged a dispatch trace on
143+
// either kernel. Handing it a logger would add a debug line to a
144+
// path that has never had one; #5282 preserves every call path's
145+
// semantics exactly, so the absence is pinned rather than assumed.
146+
const { logger, debug } = makeLogger();
147+
const reached: string[] = [];
148+
149+
await dispatchHookPropagating('kernel:ready', [async () => { reached.push('ran'); }], undefined);
150+
151+
expect(reached).toEqual(['ran']);
152+
expect(debug).not.toHaveBeenCalled();
153+
expect(logger.debug).not.toHaveBeenCalled();
154+
});
155+
156+
it('passes every argument through to each handler', async () => {
157+
const seen: unknown[][] = [];
158+
159+
await dispatchHookPropagating(
160+
'metadata:reloaded',
161+
[(...args: unknown[]) => { seen.push(args); }],
162+
undefined,
163+
['sys_user'],
164+
);
165+
166+
expect(seen).toEqual([['sys_user']]);
167+
});
168+
});
169+
170+
it('dispatches nothing, and never throws, for a hook with no subscribers', async () => {
171+
const { logger, debug, error } = makeLogger();
172+
173+
await expect(dispatchHookIsolating('kernel:shutdown', [], logger)).resolves.toBeUndefined();
174+
await expect(dispatchHookPropagating('kernel:ready', [], logger)).resolves.toBeUndefined();
175+
176+
expect(error).not.toHaveBeenCalled();
177+
expect(debug).toHaveBeenCalledTimes(2);
178+
expect(debug.mock.calls.map((c) => c[1])).toEqual([
179+
{ hook: 'kernel:shutdown', handlerCount: 0 },
180+
{ hook: 'kernel:ready', handlerCount: 0 },
181+
]);
182+
});
183+
});

0 commit comments

Comments
 (0)