fix(plugins): bound hook execution and retire stale registrations (#115695)

* fix(plugins): bound hooks and own legacy registrations

* docs(hooks): clarify internal handler ownership

* fix(cli): retain message hook shutdown deadline
This commit is contained in:
Vincent Koc
2026-07-29 16:44:05 +08:00
committed by GitHub
parent 0f9c702d8b
commit 80707db7ab
11 changed files with 329 additions and 40 deletions

View File

@@ -27,6 +27,8 @@ OpenClaw has several extension surfaces that look similar but solve different pr
Use internal hooks when you want automation that behaves like a small installed integration. Use typed plugin hooks when you need runtime lifecycle control.
Internal hook handlers are request/event handlers. They must not own long-lived timers, watchers, sockets, or clients; plugins should register a service or use the typed `gateway_start` / `gateway_stop` lifecycle instead.
## Quick start
```bash

View File

@@ -93,6 +93,14 @@ receive a cancellation signal. The hook dispatch can release its Gateway
admission while that plugin work is still in progress. Plugins that own
long-running work must provide their own cancellation and shutdown lifecycle.
Policy hooks `before_tool_call` and `before_install` use a 15-second default per
handler. A timeout fails closed: the tool call or installation is rejected
instead of continuing without a policy decision.
`gateway_stop` uses a five-second default per handler. Timed-out handlers are
logged and shutdown continues so plugin cleanup cannot consume the Gateway
process watchdog.
Outbound modifying hooks `message_sending` and `reply_payload_sending` use a
15-second default per handler. If one times out, OpenClaw logs the plugin error
and continues with the latest payload so the serialized delivery lane can

View File

@@ -40,6 +40,7 @@ async function expectGlobalRunnerState(expected: { hasRunner: boolean; registry?
}
afterEach(async () => {
vi.useRealTimers();
const mod = await importHookRunnerGlobalModule();
mod.resetGlobalHookRunner();
setActivePluginRegistry(createEmptyPluginRegistry());
@@ -131,4 +132,101 @@ describe("hook-runner-global", () => {
releasePinnedPluginChannelRegistry(gatewayRegistry);
}
});
it.each([
{
hookName: "before_tool_call" as const,
run: (runner: HookRunner) =>
runner.runBeforeToolCall({ toolName: "read", params: {} }, { toolName: "read" }),
},
{
hookName: "before_install" as const,
run: (runner: HookRunner) =>
runner.runBeforeInstall(
{
targetName: "demo",
targetType: "plugin",
sourcePath: "/tmp/demo",
sourcePathKind: "directory",
origin: "local",
request: { kind: "plugin-dir", mode: "install" },
builtinScan: {
status: "ok",
scannedFiles: 0,
critical: 0,
warn: 0,
info: 0,
findings: [],
},
},
{ origin: "local", targetType: "plugin", requestKind: "plugin-dir" },
),
},
])("fails closed when a default-bounded $hookName handler hangs", async ({ hookName, run }) => {
vi.useFakeTimers();
let releaseHandler: (() => void) | undefined;
const registry = createMockPluginRegistry([
{
hookName,
pluginId: "hanging-policy",
handler: () =>
new Promise<void>((resolve) => {
releaseHandler = resolve;
}),
},
]);
const mod = await importHookRunnerGlobalModule();
setActivePluginRegistry(registry);
mod.initializeGlobalHookRunner(registry);
const pending = run(expectGlobalHookRunner(mod.getGlobalHookRunner()));
try {
expect(vi.getTimerCount()).toBeGreaterThan(0);
const rejection = expect(pending).rejects.toThrow(
`${hookName} handler from hanging-policy failed: timed out after 15000ms`,
);
await vi.advanceTimersByTimeAsync(15_000);
await rejection;
} finally {
releaseHandler?.();
await pending.catch(() => undefined);
}
});
it("bounds gateway_stop handlers and lets shutdown continue", async () => {
vi.useFakeTimers();
let releaseHandler: (() => void) | undefined;
const registry = createMockPluginRegistry([
{
hookName: "gateway_stop",
pluginId: "hanging-shutdown",
handler: () =>
new Promise<void>((resolve) => {
releaseHandler = resolve;
}),
},
]);
const mod = await importHookRunnerGlobalModule();
setActivePluginRegistry(registry);
mod.initializeGlobalHookRunner(registry);
const pending = mod.runGlobalGatewayStopSafely({
event: { reason: "test shutdown" },
ctx: {},
});
try {
expect(vi.getTimerCount()).toBeGreaterThan(0);
let settled = false;
void pending.then(() => {
settled = true;
});
await vi.advanceTimersByTimeAsync(4_999);
expect(settled).toBe(false);
await vi.advanceTimersByTimeAsync(1);
await expect(pending).resolves.toBeUndefined();
} finally {
releaseHandler?.();
await pending;
}
});
});

View File

@@ -156,9 +156,16 @@ const DEFAULT_VOID_HOOK_TIMEOUT_MS_BY_HOOK: Partial<Record<PluginHookName, numbe
after_compaction: 30_000,
skill_changed: 30_000,
skill_proposal_changed: 30_000,
// Shutdown hooks share the Gateway's five-second teardown budget. They fail
// open after logging so one plugin cannot consume the process watchdog.
gateway_stop: 5_000,
};
const DEFAULT_MODIFYING_HOOK_TIMEOUT_MS_BY_HOOK: Partial<Record<PluginHookName, number>> = {
before_agent_run: 15_000,
// Policy hooks fail closed in the global runner. A bounded timeout turns a
// stalled policy process into a denial instead of freezing the operation.
before_install: 15_000,
before_tool_call: 15_000,
// Terminal finalization hooks sit on the runner's completion path. A hung
// handler must not freeze final delivery or keep compaction retry recovery
// unresolved; timeout fail-opens with the original final answer.

View File

@@ -0,0 +1,66 @@
import {
registerInternalHook,
unregisterInternalHook,
type InternalHookHandler,
} from "../hooks/internal-hooks.js";
import { resolveGlobalSingleton } from "../shared/global-singleton.js";
export type LegacyPluginInternalHookRegistration = {
event: string;
handler: InternalHookHandler;
};
export type LegacyPluginInternalHookState = Map<string, LegacyPluginInternalHookRegistration[]>;
const LEGACY_PLUGIN_INTERNAL_HOOKS_KEY = Symbol.for("openclaw.activePluginHookRegistrations");
const registrations = resolveGlobalSingleton<LegacyPluginInternalHookState>(
LEGACY_PLUGIN_INTERNAL_HOOKS_KEY,
() => new Map(),
);
function cloneRegistrations(
values: readonly LegacyPluginInternalHookRegistration[],
): LegacyPluginInternalHookRegistration[] {
return values.map((registration) => ({ ...registration }));
}
export function replaceLegacyPluginInternalHook(
name: string,
nextRegistrations: readonly LegacyPluginInternalHookRegistration[],
): LegacyPluginInternalHookRegistration[] {
const previousRegistrations = cloneRegistrations(registrations.get(name) ?? []);
for (const registration of registrations.get(name) ?? []) {
unregisterInternalHook(registration.event, registration.handler);
}
for (const registration of nextRegistrations) {
registerInternalHook(registration.event, registration.handler);
}
if (nextRegistrations.length === 0) {
registrations.delete(name);
} else {
registrations.set(name, cloneRegistrations(nextRegistrations));
}
return previousRegistrations;
}
export function clearLegacyPluginInternalHooks(): void {
for (const name of registrations.keys()) {
replaceLegacyPluginInternalHook(name, []);
}
}
export function snapshotLegacyPluginInternalHooks(): LegacyPluginInternalHookState {
return new Map(
[...registrations].map(([name, hookRegistrations]) => [
name,
cloneRegistrations(hookRegistrations),
]),
);
}
export function restoreLegacyPluginInternalHooks(state: LegacyPluginInternalHookState): void {
clearLegacyPluginInternalHooks();
for (const [name, hookRegistrations] of state) {
replaceLegacyPluginInternalHook(name, hookRegistrations);
}
}

View File

@@ -25,6 +25,7 @@ import { clearEmbeddingProviders } from "./embedding-providers.js";
import { initializeGlobalHookRunner } from "./hook-runner-global.js";
import { collectPluginManifestCompatCodes } from "./installed-plugin-index-record-builder.js";
import { clearPluginInteractiveHandlers } from "./interactive-registry.js";
import { clearLegacyPluginInternalHooks } from "./legacy-internal-hook-state.js";
import { createPluginRecord } from "./loader-records.js";
import type { PluginLoadOptions, PluginRuntimeSubagentMode } from "./loader-types.js";
import type { PluginManifestRecord, PluginManifestRegistry } from "./manifest-registry.js";
@@ -172,6 +173,10 @@ export function clearActivatedPluginRuntimeState(): void {
clearCompactionProviders();
clearDetachedTaskLifecycleRuntimeRegistration();
clearPluginInteractiveHandlers();
// Legacy api.registerHook callbacks are process-global compatibility state.
// Retire them with the active registry so disabled or removed plugins cannot
// keep running.
clearLegacyPluginInternalHooks();
clearEmbeddingProviders();
clearMemoryEmbeddingProviders();
clearMemoryPluginState();

View File

@@ -1789,6 +1789,13 @@ describe("loadOpenClawPlugins", () => {
api.registerAgentToolResultMiddleware(() => undefined, {
runtimes: ["openclaw"],
});
api.registerHook(
"gateway:startup",
(event) => {
event.messages.push("rollback-hook-fired");
},
{ name: "reload-rollback-hook" },
);
api.on("gateway_stop", async () => {});
},
};`,
@@ -1811,7 +1818,7 @@ describe("loadOpenClawPlugins", () => {
};
const activeRegistry = loadOpenClawPlugins(loadOptions);
const expectRegistrationsIntact = () => {
const expectRegistrationsIntact = async () => {
expect(getActivePluginRegistry()).toBe(activeRegistry);
expect(getRegisteredAgentHarness("codex")).toBeDefined();
expect(getPluginCommandSpecs().map((entry) => entry.name)).toEqual(["pair"]);
@@ -1820,8 +1827,11 @@ describe("loadOpenClawPlugins", () => {
]);
expect(activeRegistry.agentToolResultMiddlewares).toHaveLength(1);
expect(activeRegistry.typedHooks.map((entry) => entry.hookName)).toEqual(["gateway_stop"]);
const event = createInternalHookEvent("gateway", "startup", "gateway:startup");
await triggerInternalHook(event);
expect(event.messages).toEqual(["rollback-hook-fired"]);
};
expectRegistrationsIntact();
await expectRegistrationsIntact();
const manifestRegistry = await import("./manifest-registry.js");
const manifestSpy = vi
@@ -1832,7 +1842,7 @@ describe("loadOpenClawPlugins", () => {
try {
expect(() => loadOpenClawPlugins(loadOptions)).toThrow("corrupt plugin manifest");
expectRegistrationsIntact();
await expectRegistrationsIntact();
} finally {
manifestSpy.mockRestore();
}
@@ -1858,7 +1868,7 @@ describe("loadOpenClawPlugins", () => {
onlyPluginIds: ["reload-rollback", "reload-rollback-failure"],
}),
).toThrow("plugin load failed: reload-rollback-failure: Error: register failed");
expectRegistrationsIntact();
await expectRegistrationsIntact();
});
it("rejects malformed plugin agent harness registrations", () => {
@@ -1980,6 +1990,72 @@ describe("loadOpenClawPlugins", () => {
clearInternalHooks();
});
it.each(["disabled", "removed"] as const)(
"clears legacy internal hooks when their plugin is %s",
async (nextState) => {
useNoBundledPlugins();
const plugin = writePlugin({
id: "internal-hook-lifecycle",
filename: "internal-hook-lifecycle.cjs",
body: `module.exports = {
id: "internal-hook-lifecycle",
register(api) {
api.registerHook(
"gateway:startup",
(event) => {
event.messages.push("legacy-hook-fired");
},
{ name: "legacy-lifecycle-hook" },
);
},
};`,
});
clearInternalHooks();
loadOpenClawPlugins({
cache: false,
workspaceDir: plugin.dir,
config: {
plugins: {
load: { paths: [plugin.file] },
allow: ["internal-hook-lifecycle"],
},
},
});
const activeEvent = createInternalHookEvent("gateway", "startup", "gateway:startup");
await triggerInternalHook(activeEvent);
expect(activeEvent.messages).toEqual(["legacy-hook-fired"]);
loadOpenClawPlugins({
cache: false,
workspaceDir: plugin.dir,
config: {
plugins:
nextState === "disabled"
? {
load: { paths: [plugin.file] },
allow: ["internal-hook-lifecycle"],
entries: {
"internal-hook-lifecycle": {
enabled: false,
},
},
}
: {
allow: [],
},
},
});
const retiredEvent = createInternalHookEvent("gateway", "startup", "gateway:startup");
await triggerInternalHook(retiredEvent);
expect(retiredEvent.messages).toStrictEqual([]);
clearInternalHooks();
},
);
it("injects plugin config into internal hook event context", async () => {
useNoBundledPlugins();
const plugin = writePlugin({

View File

@@ -915,6 +915,49 @@ describe("loadOpenClawPlugins", () => {
expect(getDetachedTaskLifecycleRuntimeRegistration()?.pluginId).toBe("cached-detached-runtime");
});
it("restores cached legacy internal hook registrations on cache hits", async () => {
useNoBundledPlugins();
const plugin = writePlugin({
id: "cached-legacy-hook",
filename: "cached-legacy-hook.cjs",
body: `module.exports = {
id: "cached-legacy-hook",
register(api) {
api.registerHook(
"gateway:startup",
(event) => {
event.messages.push("cached-hook-fired");
},
{ name: "cached-legacy-hook" },
);
},
};`,
});
const loadOptions = {
workspaceDir: plugin.dir,
config: {
plugins: {
load: { paths: [plugin.file] },
allow: ["cached-legacy-hook"],
},
},
onlyPluginIds: ["cached-legacy-hook"],
} satisfies Parameters<typeof loadOpenClawPlugins>[0];
loadOpenClawPlugins(loadOptions);
const firstEvent = createInternalHookEvent("gateway", "startup", "gateway:startup");
await triggerInternalHook(firstEvent);
expect(firstEvent.messages).toEqual(["cached-hook-fired"]);
clearInternalHooks();
loadOpenClawPlugins(loadOptions);
const cachedEvent = createInternalHookEvent("gateway", "startup", "gateway:startup");
await triggerInternalHook(cachedEvent);
expect(cachedEvent.messages).toEqual(["cached-hook-fired"]);
});
it("restores cached command and interactive handler registrations on cache hits", () => {
useNoBundledPlugins();
const plugin = writePlugin({

View File

@@ -20,6 +20,11 @@ import {
listPluginInteractiveHandlers,
restorePluginInteractiveHandlers,
} from "./interactive-registry.js";
import {
restoreLegacyPluginInternalHooks,
snapshotLegacyPluginInternalHooks,
type LegacyPluginInternalHookState,
} from "./legacy-internal-hook-state.js";
import {
listRegisteredMemoryEmbeddingProviders,
restoreRegisteredMemoryEmbeddingProviders,
@@ -44,6 +49,7 @@ export type PluginProcessGlobalState = {
detachedTaskRuntimeRegistration: ReturnType<typeof getDetachedTaskLifecycleRuntimeRegistration>;
embeddingProviders: ReturnType<typeof listRegisteredEmbeddingProviders>;
interactiveHandlers: ReturnType<typeof listPluginInteractiveHandlers>;
legacyInternalHooks: LegacyPluginInternalHookState;
memoryCapability: ReturnType<typeof getMemoryCapabilityRegistration>;
memoryCorpusSupplements: ReturnType<typeof listMemoryCorpusSupplements>;
memoryEmbeddingProviders: ReturnType<typeof listRegisteredMemoryEmbeddingProviders>;
@@ -60,6 +66,7 @@ export function snapshotPluginProcessGlobalState(): PluginProcessGlobalState {
detachedTaskRuntimeRegistration: getDetachedTaskLifecycleRuntimeRegistration(),
embeddingProviders: listRegisteredEmbeddingProviders(),
interactiveHandlers: listPluginInteractiveHandlers(),
legacyInternalHooks: snapshotLegacyPluginInternalHooks(),
memoryCapability: getMemoryCapabilityRegistration(),
memoryCorpusSupplements: listMemoryCorpusSupplements(),
memoryEmbeddingProviders: listRegisteredMemoryEmbeddingProviders(),
@@ -76,6 +83,7 @@ export function restorePluginProcessGlobalState(state: PluginProcessGlobalState)
restoreDetachedTaskLifecycleRuntimeRegistration(state.detachedTaskRuntimeRegistration);
restoreRegisteredEmbeddingProviders(state.embeddingProviders);
restorePluginInteractiveHandlers(state.interactiveHandlers);
restoreLegacyPluginInternalHooks(state.legacyInternalHooks);
restoreRegisteredMemoryEmbeddingProviders(state.memoryEmbeddingProviders);
restoreMemoryPluginState({
capability: state.memoryCapability,

View File

@@ -2,9 +2,8 @@ import path from "node:path";
import { uniqueValues } from "@openclaw/normalization-core/string-normalization";
import { normalizeStringEntries } from "@openclaw/normalization-core/string-normalization";
import type { AnyAgentTool } from "../agents/tools/common.js";
import { registerInternalHook, unregisterInternalHook } from "../hooks/internal-hooks.js";
import type { InternalHookHandler } from "../hooks/internal-hooks.js";
import type { HookEntry } from "../hooks/types.js";
import { resolveGlobalSingleton } from "../shared/global-singleton.js";
import { withTimeout } from "../utils/with-timeout.js";
import type { AgentToolResultMiddleware } from "./agent-tool-result-middleware-types.js";
import {
@@ -14,6 +13,10 @@ import {
import { CODEX_APP_SERVER_EXTENSION_RUNTIME_ID } from "./codex-app-server-extension-factory.js";
import type { CodexAppServerExtensionFactory } from "./codex-app-server-extension-types.js";
import { getPluginCompatRecord } from "./compat/registry.js";
import {
replaceLegacyPluginInternalHook,
type LegacyPluginInternalHookRegistration,
} from "./legacy-internal-hook-state.js";
import {
resolveTypedHookTimeoutMs,
type PluginRegistryState,
@@ -46,11 +49,6 @@ import type {
const LEGACY_DEACTIVATE_HOOK_ALIAS_COMPAT = getPluginCompatRecord("legacy-deactivate-hook-alias");
const LEGACY_SUBAGENT_SPAWNING_HOOK_COMPAT = getPluginCompatRecord("legacy-subagent-spawning-hook");
const ACTIVE_PLUGIN_HOOK_REGISTRATIONS_KEY = Symbol.for("openclaw.activePluginHookRegistrations");
const activePluginHookRegistrations = resolveGlobalSingleton<
Map<string, Array<{ event: string; handler: Parameters<typeof registerInternalHook>[1] }>>
>(ACTIVE_PLUGIN_HOOK_REGISTRATIONS_KEY, () => new Map());
function formatLegacyDeactivateHookAliasDiagnostic(): string {
const removeAfter =
LEGACY_DEACTIVATE_HOOK_ALIAS_COMPAT.removeAfter ?? "a future breaking release";
@@ -288,7 +286,7 @@ export function createToolHookRegistrars(state: PluginRegistryState) {
const registerHook = (
record: PluginRecord,
events: string | string[],
handler: Parameters<typeof registerInternalHook>[1],
handler: InternalHookHandler,
opts: OpenClawPluginHookOptions | undefined,
config: OpenClawPluginApi["config"],
pluginConfig: unknown,
@@ -353,14 +351,7 @@ export function createToolHookRegistrars(state: PluginRegistryState) {
) {
return;
}
const previousRegistrations = activePluginHookRegistrations.get(hookName) ?? [];
for (const registration of previousRegistrations) {
unregisterInternalHook(registration.event, registration.handler);
}
const nextRegistrations: Array<{
event: string;
handler: Parameters<typeof registerInternalHook>[1];
}> = [];
const nextRegistrations: LegacyPluginInternalHookRegistration[] = [];
for (const event of normalizedEvents) {
const wrappedHandler: typeof handler = async (evt) => {
const context = evt.context;
@@ -378,12 +369,11 @@ export function createToolHookRegistrars(state: PluginRegistryState) {
}
}
};
registerInternalHook(event, wrappedHandler);
nextRegistrations.push({ event, handler: wrappedHandler });
}
activePluginHookRegistrations.set(hookName, nextRegistrations);
const previousRegistrations = replaceLegacyPluginInternalHook(hookName, nextRegistrations);
const rollbackEntries = pluginHookRollback.get(record.id) ?? [];
rollbackEntries.push({ name: hookName, previousRegistrations: [...previousRegistrations] });
rollbackEntries.push({ name: hookName, previousRegistrations });
pluginHookRollback.set(record.id, rollbackEntries);
};
@@ -471,18 +461,7 @@ export function createToolHookRegistrars(state: PluginRegistryState) {
const rollbackHooks = (pluginId: string) => {
const hookRollbackEntries = pluginHookRollback.get(pluginId) ?? [];
for (const entry of hookRollbackEntries.toReversed()) {
const activeRegistrations = activePluginHookRegistrations.get(entry.name) ?? [];
for (const registration of activeRegistrations) {
unregisterInternalHook(registration.event, registration.handler);
}
if (entry.previousRegistrations.length === 0) {
activePluginHookRegistrations.delete(entry.name);
continue;
}
for (const registration of entry.previousRegistrations) {
registerInternalHook(registration.event, registration.handler);
}
activePluginHookRegistrations.set(entry.name, [...entry.previousRegistrations]);
replaceLegacyPluginInternalHook(entry.name, entry.previousRegistrations);
}
pluginHookRollback.delete(pluginId);
};

View File

@@ -1,4 +1,4 @@
import type { registerInternalHook } from "../hooks/internal-hooks.js";
import type { LegacyPluginInternalHookRegistration } from "./legacy-internal-hook-state.js";
import type { PluginDiagnostic } from "./manifest-types.js";
import { createModelCatalogRegistrationHandlers } from "./model-catalog-registration.js";
import { createEmptyPluginRegistry } from "./registry-empty.js";
@@ -83,10 +83,7 @@ export function createPluginRegistryState(registryParams: PluginRegistryParams)
string,
Array<{
name: string;
previousRegistrations: Array<{
event: string;
handler: Parameters<typeof registerInternalHook>[1];
}>;
previousRegistrations: LegacyPluginInternalHookRegistration[];
}>
>(),
pluginsWithChannelRegistrationConflict: new Set<string>(),