From 10161c2d79ab299d094dbc1f1eedd45a4121d63e Mon Sep 17 00:00:00 2001 From: Josh Avant <830519+joshavant@users.noreply.github.com> Date: Wed, 25 Mar 2026 00:11:13 -0500 Subject: [PATCH] Plugins: enforce terminal hook decision semantics for tool/message guards (#54241) * Plugins: enforce terminal hook decision policies * Tests: assert terminal hook behavior in integration paths * Docs: clarify terminal hook decision semantics * Docs: add hook guard semantics to plugin guides * Tests: isolate outbound format label expectations * changelog Signed-off-by: joshavant <830519+joshavant@users.noreply.github.com> --------- Signed-off-by: joshavant <830519+joshavant@users.noreply.github.com> --- CHANGELOG.md | 1 + docs/concepts/agent-loop.md | 7 + docs/plugins/building-plugins.md | 9 + docs/plugins/sdk-overview.md | 7 + docs/tools/plugin.md | 9 + ...s.before-tool-call.integration.e2e.test.ts | 47 +++- src/infra/outbound/deliver.test.ts | 55 ++++- src/infra/outbound/format.test.ts | 8 +- src/plugins/hooks.security.test.ts | 232 ++++++++++++++++++ src/plugins/hooks.ts | 87 +++++-- 10 files changed, 435 insertions(+), 27 deletions(-) create mode 100644 src/plugins/hooks.security.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 10bbf513631..85c322005d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ Docs: https://docs.openclaw.ai - Gateway/ports: parse Docker Compose-style `OPENCLAW_GATEWAY_PORT` host publish values correctly without reviving the legacy `CLAWDBOT_GATEWAY_PORT` override. (#44083) Thanks @bebule. - Feishu/MSTeams message tool: keep provider-native `card` payloads optional in merged tool schemas so media-only sends stop failing validation before channel runtime dispatch. (#53715) Thanks @lndyzwdxhs. - Feishu/startup: keep `requireMention` enforcement strict when bot identity startup probes fail, raise the startup bot-info timeout to 30s, and add cancellable background identity recovery so mention-gated groups recover without noisy fallback. (#43788) Thanks @lefarcen. +- Plugins: enforce terminal hook decision semantics for tool/message guards (#54241) Thanks @joshavant. ## 2026.3.23 diff --git a/docs/concepts/agent-loop.md b/docs/concepts/agent-loop.md index bf60b23f1d7..b228d7ac124 100644 --- a/docs/concepts/agent-loop.md +++ b/docs/concepts/agent-loop.md @@ -92,6 +92,13 @@ These run inside the agent loop or gateway pipeline: - **`session_start` / `session_end`**: session lifecycle boundaries. - **`gateway_start` / `gateway_stop`**: gateway lifecycle events. +Hook decision rules for outbound/tool guards: + +- `before_tool_call`: `{ block: true }` is terminal and stops lower-priority handlers. +- `before_tool_call`: `{ block: false }` is a no-op and does not clear a prior block. +- `message_sending`: `{ cancel: true }` is terminal and stops lower-priority handlers. +- `message_sending`: `{ cancel: false }` is a no-op and does not clear a prior cancel. + See [Plugin hooks](/plugins/architecture#provider-runtime-hooks) for the hook API and registration details. ## Streaming + partial replies diff --git a/docs/plugins/building-plugins.md b/docs/plugins/building-plugins.md index a41572ac309..d2244453c57 100644 --- a/docs/plugins/building-plugins.md +++ b/docs/plugins/building-plugins.md @@ -144,6 +144,15 @@ A single plugin can register any number of capabilities via the `api` object: For the full registration API, see [SDK Overview](/plugins/sdk-overview#registration-api). +Hook guard semantics to keep in mind: + +- `before_tool_call`: `{ block: true }` is terminal and stops lower-priority handlers. +- `before_tool_call`: `{ block: false }` is treated as no decision. +- `message_sending`: `{ cancel: true }` is terminal and stops lower-priority handlers. +- `message_sending`: `{ cancel: false }` is treated as no decision. + +See [SDK Overview hook decision semantics](/plugins/sdk-overview#hook-decision-semantics) for details. + ## Registering agent tools Tools are typed functions the LLM can call. They can be required (always diff --git a/docs/plugins/sdk-overview.md b/docs/plugins/sdk-overview.md index 0ba16d82b4e..eda2085cc50 100644 --- a/docs/plugins/sdk-overview.md +++ b/docs/plugins/sdk-overview.md @@ -152,6 +152,13 @@ methods: | `api.on(hookName, handler, opts?)` | Typed lifecycle hook | | `api.onConversationBindingResolved(handler)` | Conversation binding callback | +### Hook decision semantics + +- `before_tool_call`: returning `{ block: true }` is terminal. Once any handler sets it, lower-priority handlers are skipped. +- `before_tool_call`: returning `{ block: false }` is treated as no decision (same as omitting `block`), not as an override. +- `message_sending`: returning `{ cancel: true }` is terminal. Once any handler sets it, lower-priority handlers are skipped. +- `message_sending`: returning `{ cancel: false }` is treated as no decision (same as omitting `cancel`), not as an override. + ### API object fields | Field | Type | Description | diff --git a/docs/tools/plugin.md b/docs/tools/plugin.md index 979cb9b698e..83bc123a4cf 100644 --- a/docs/tools/plugin.md +++ b/docs/tools/plugin.md @@ -258,6 +258,15 @@ Common registration methods: | `registerContextEngine` | Context engine | | `registerService` | Background service | +Hook guard behavior for typed lifecycle hooks: + +- `before_tool_call`: `{ block: true }` is terminal; lower-priority handlers are skipped. +- `before_tool_call`: `{ block: false }` is a no-op and does not clear an earlier block. +- `message_sending`: `{ cancel: true }` is terminal; lower-priority handlers are skipped. +- `message_sending`: `{ cancel: false }` is a no-op and does not clear an earlier cancel. + +For full typed hook behavior, see [SDK Overview](/plugins/sdk-overview#hook-decision-semantics). + ## Related - [Building Plugins](/plugins/building-plugins) — create your own plugin diff --git a/src/agents/pi-tools.before-tool-call.integration.e2e.test.ts b/src/agents/pi-tools.before-tool-call.integration.e2e.test.ts index 40a2a7bee4b..e3b10844c70 100644 --- a/src/agents/pi-tools.before-tool-call.integration.e2e.test.ts +++ b/src/agents/pi-tools.before-tool-call.integration.e2e.test.ts @@ -4,7 +4,9 @@ import { initializeGlobalHookRunner, resetGlobalHookRunner, } from "../plugins/hook-runner-global.js"; -import { createMockPluginRegistry } from "../plugins/hooks.test-helpers.js"; +import { addTestHook, createMockPluginRegistry } from "../plugins/hooks.test-helpers.js"; +import { createEmptyPluginRegistry } from "../plugins/registry.js"; +import type { PluginHookRegistration } from "../plugins/types.js"; type ToolDefinitionAdapterModule = typeof import("./pi-tool-definition-adapter.js"); type PiToolsAbortModule = typeof import("./pi-tools.abort.js"); @@ -39,6 +41,12 @@ beforeEach(async () => { type BeforeToolCallHandlerMock = ReturnType; +type BeforeToolCallHookInstall = { + pluginId: string; + priority?: number; + handler: BeforeToolCallHandlerMock; +}; + function installBeforeToolCallHook(params?: { enabled?: boolean; runBeforeToolCallImpl?: (...args: unknown[]) => unknown; @@ -54,6 +62,21 @@ function installBeforeToolCallHook(params?: { return handler; } +function installBeforeToolCallHooks(hooks: BeforeToolCallHookInstall[]): void { + resetGlobalHookRunner(); + const registry = createEmptyPluginRegistry(); + for (const hook of hooks) { + addTestHook({ + registry, + pluginId: hook.pluginId, + hookName: "before_tool_call", + handler: hook.handler as PluginHookRegistration["handler"], + priority: hook.priority, + }); + } + initializeGlobalHookRunner(registry); +} + describe("before_tool_call hook integration", () => { let beforeToolCallHook: BeforeToolCallHandlerMock; @@ -122,6 +145,28 @@ describe("before_tool_call hook integration", () => { expect(execute).not.toHaveBeenCalled(); }); + it("does not execute lower-priority hooks after block=true", async () => { + const high = vi.fn().mockResolvedValue({ block: true, blockReason: "blocked-high" }); + const low = vi.fn().mockResolvedValue({ params: { shouldNotApply: true } }); + installBeforeToolCallHooks([ + { pluginId: "high", priority: 100, handler: high }, + { pluginId: "low", priority: 0, handler: low }, + ]); + + const execute = vi.fn().mockResolvedValue({ content: [], details: { ok: true } }); + // oxlint-disable-next-line typescript/no-explicit-any + const tool = wrapToolWithBeforeToolCallHook({ name: "exec", execute } as any); + const extensionContext = {} as Parameters[3]; + + await expect( + tool.execute("call-stop", { cmd: "rm -rf /" }, undefined, extensionContext), + ).rejects.toThrow("blocked-high"); + + expect(high).toHaveBeenCalledTimes(1); + expect(low).not.toHaveBeenCalled(); + expect(execute).not.toHaveBeenCalled(); + }); + it("continues execution when hook throws", async () => { beforeToolCallHook = installBeforeToolCallHook({ runBeforeToolCallImpl: async () => { diff --git a/src/infra/outbound/deliver.test.ts b/src/infra/outbound/deliver.test.ts index 4e659ca3d39..0069f136de5 100644 --- a/src/infra/outbound/deliver.test.ts +++ b/src/infra/outbound/deliver.test.ts @@ -7,7 +7,11 @@ import { whatsappOutbound, } from "../../../test/channel-outbounds.js"; import type { OpenClawConfig } from "../../config/config.js"; +import { createHookRunner } from "../../plugins/hooks.js"; +import { addTestHook } from "../../plugins/hooks.test-helpers.js"; +import { createEmptyPluginRegistry } from "../../plugins/registry.js"; import { setActivePluginRegistry } from "../../plugins/runtime.js"; +import type { PluginHookRegistration } from "../../plugins/types.js"; import { createOutboundTestPlugin, createTestRegistry } from "../../test-utils/channel-plugins.js"; import { withEnvAsync } from "../../test-utils/env.js"; import { createIMessageTestPlugin } from "../../test-utils/imessage-test-plugin.js"; @@ -19,8 +23,11 @@ const mocks = vi.hoisted(() => ({ })); const hookMocks = vi.hoisted(() => ({ runner: { - hasHooks: vi.fn(() => false), - runMessageSent: vi.fn(async () => {}), + hasHooks: vi.fn<(_hookName?: string) => boolean>(() => false), + runMessageSending: vi.fn<(event: unknown, ctx: unknown) => Promise>( + async () => undefined, + ), + runMessageSent: vi.fn<(event: unknown, ctx: unknown) => Promise>(async () => {}), }, })); const internalHookMocks = vi.hoisted(() => ({ @@ -210,6 +217,8 @@ describe("deliverOutboundPayloads", () => { mocks.appendAssistantMessageToSessionTranscript.mockClear(); hookMocks.runner.hasHooks.mockClear(); hookMocks.runner.hasHooks.mockReturnValue(false); + hookMocks.runner.runMessageSending.mockClear(); + hookMocks.runner.runMessageSending.mockResolvedValue(undefined); hookMocks.runner.runMessageSent.mockClear(); hookMocks.runner.runMessageSent.mockResolvedValue(undefined); internalHookMocks.createInternalHookEvent.mockClear(); @@ -1031,6 +1040,48 @@ describe("deliverOutboundPayloads", () => { ); }); + it("short-circuits lower-priority message_sending hooks after cancel=true", async () => { + const hookRegistry = createEmptyPluginRegistry(); + const high = vi.fn().mockResolvedValue({ cancel: true, content: "blocked" }); + const low = vi.fn().mockResolvedValue({ cancel: false, content: "override" }); + addTestHook({ + registry: hookRegistry, + pluginId: "high", + hookName: "message_sending", + handler: high as PluginHookRegistration["handler"], + priority: 100, + }); + addTestHook({ + registry: hookRegistry, + pluginId: "low", + hookName: "message_sending", + handler: low as PluginHookRegistration["handler"], + priority: 0, + }); + const realRunner = createHookRunner(hookRegistry); + hookMocks.runner.hasHooks.mockImplementation((hookName?: string) => + realRunner.hasHooks((hookName ?? "") as never), + ); + hookMocks.runner.runMessageSending.mockImplementation((event, ctx) => + realRunner.runMessageSending(event as never, ctx as never), + ); + + const sendWhatsApp = vi.fn().mockResolvedValue({ messageId: "w1", toJid: "jid" }); + await deliverOutboundPayloads({ + cfg: {}, + channel: "whatsapp", + to: "+1555", + payloads: [{ text: "hello" }], + deps: { sendWhatsApp }, + }); + + expect(hookMocks.runner.runMessageSending).toHaveBeenCalledTimes(1); + expect(high).toHaveBeenCalledTimes(1); + expect(low).not.toHaveBeenCalled(); + expect(sendWhatsApp).not.toHaveBeenCalled(); + expect(hookMocks.runner.runMessageSent).not.toHaveBeenCalled(); + }); + it("emits message_sent success for sendPayload deliveries", async () => { hookMocks.runner.hasHooks.mockReturnValue(true); const sendPayload = vi.fn().mockResolvedValue({ channel: "matrix", messageId: "mx-1" }); diff --git a/src/infra/outbound/format.test.ts b/src/infra/outbound/format.test.ts index db30cd4c511..ea0751a42fb 100644 --- a/src/infra/outbound/format.test.ts +++ b/src/infra/outbound/format.test.ts @@ -1,10 +1,16 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; import { buildOutboundDeliveryJson, formatGatewaySummary, formatOutboundDeliverySummary, } from "./format.js"; +const getChannelPluginMock = vi.hoisted(() => vi.fn((_channel: unknown) => undefined)); + +vi.mock("../../channels/plugins/index.js", () => ({ + getChannelPlugin: getChannelPluginMock, +})); + describe("formatOutboundDeliverySummary", () => { it("formats fallback and provider-specific detail variants", () => { const cases = [ diff --git a/src/plugins/hooks.security.test.ts b/src/plugins/hooks.security.test.ts new file mode 100644 index 00000000000..5b8d9dc1370 --- /dev/null +++ b/src/plugins/hooks.security.test.ts @@ -0,0 +1,232 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { createHookRunner } from "./hooks.js"; +import { addTestHook } from "./hooks.test-helpers.js"; +import { createEmptyPluginRegistry, type PluginRegistry } from "./registry.js"; +import type { + PluginHookBeforeToolCallResult, + PluginHookMessageSendingResult, + PluginHookRegistration, +} from "./types.js"; + +function addBeforeToolCallHook( + registry: PluginRegistry, + pluginId: string, + handler: () => PluginHookBeforeToolCallResult | Promise, + priority?: number, +) { + addTestHook({ + registry, + pluginId, + hookName: "before_tool_call", + handler: handler as PluginHookRegistration["handler"], + priority, + }); +} + +function addMessageSendingHook( + registry: PluginRegistry, + pluginId: string, + handler: () => PluginHookMessageSendingResult | Promise, + priority?: number, +) { + addTestHook({ + registry, + pluginId, + hookName: "message_sending", + handler: handler as PluginHookRegistration["handler"], + priority, + }); +} + +const toolEvent = { toolName: "bash", params: { command: "echo hello" } }; +const toolCtx = { toolName: "bash" }; +const messageEvent = { to: "user-1", content: "hello" }; +const messageCtx = { channelId: "telegram" }; + +describe("before_tool_call terminal block semantics", () => { + let registry: PluginRegistry; + + beforeEach(() => { + registry = createEmptyPluginRegistry(); + }); + + it("keeps block=true when a lower-priority hook returns block=false", async () => { + addBeforeToolCallHook(registry, "high", () => ({ block: true, blockReason: "dangerous" }), 100); + addBeforeToolCallHook(registry, "low", () => ({ block: false }), 10); + + const runner = createHookRunner(registry); + const result = await runner.runBeforeToolCall(toolEvent, toolCtx); + + expect(result?.block).toBe(true); + expect(result?.blockReason).toBe("dangerous"); + }); + + it("treats explicit block=false as no-op when no prior hook blocked", async () => { + addBeforeToolCallHook(registry, "single", () => ({ block: false }), 10); + + const runner = createHookRunner(registry); + const result = await runner.runBeforeToolCall(toolEvent, toolCtx); + + expect(result?.block).toBeUndefined(); + }); + + it("treats passive handler output as no-op for prior block", async () => { + addBeforeToolCallHook(registry, "high", () => ({ block: true, blockReason: "blocked" }), 100); + addBeforeToolCallHook(registry, "passive", () => ({}), 10); + + const runner = createHookRunner(registry); + const result = await runner.runBeforeToolCall(toolEvent, toolCtx); + + expect(result?.block).toBe(true); + expect(result?.blockReason).toBe("blocked"); + }); + + it("short-circuits lower-priority hooks after block=true", async () => { + const high = vi.fn().mockReturnValue({ block: true, blockReason: "stop" }); + const low = vi.fn().mockReturnValue({ params: { injected: true } }); + addBeforeToolCallHook(registry, "high", high, 100); + addBeforeToolCallHook(registry, "low", low, 10); + + const runner = createHookRunner(registry); + const result = await runner.runBeforeToolCall(toolEvent, toolCtx); + + expect(result?.block).toBe(true); + expect(high).toHaveBeenCalledTimes(1); + expect(low).not.toHaveBeenCalled(); + }); + + it("preserves deterministic same-priority registration order when terminal hook runs first", async () => { + const first = vi.fn().mockReturnValue({ block: true, blockReason: "first" }); + const second = vi.fn().mockReturnValue({ block: true, blockReason: "second" }); + addBeforeToolCallHook(registry, "first", first, 50); + addBeforeToolCallHook(registry, "second", second, 50); + + const runner = createHookRunner(registry); + const result = await runner.runBeforeToolCall(toolEvent, toolCtx); + + expect(result?.block).toBe(true); + expect(result?.blockReason).toBe("first"); + expect(first).toHaveBeenCalledTimes(1); + expect(second).not.toHaveBeenCalled(); + }); + + it("stops before lower-priority throwing hooks when catchErrors is false", async () => { + addBeforeToolCallHook(registry, "high", () => ({ block: true, blockReason: "guard" }), 100); + const low = vi.fn().mockImplementation(() => { + throw new Error("should not run"); + }); + addBeforeToolCallHook(registry, "low", low, 10); + + const runner = createHookRunner(registry, { catchErrors: false }); + const result = await runner.runBeforeToolCall(toolEvent, toolCtx); + + expect(result?.block).toBe(true); + expect(low).not.toHaveBeenCalled(); + }); + + it("respects block from a middle hook in a multi-handler chain", async () => { + const low = vi.fn().mockReturnValue({ block: false }); + addBeforeToolCallHook(registry, "high-passive", () => ({}), 100); + addBeforeToolCallHook( + registry, + "middle-block", + () => ({ block: true, blockReason: "mid" }), + 50, + ); + addBeforeToolCallHook(registry, "low-false", low, 0); + + const runner = createHookRunner(registry); + const result = await runner.runBeforeToolCall(toolEvent, toolCtx); + + expect(result?.block).toBe(true); + expect(result?.blockReason).toBe("mid"); + expect(low).not.toHaveBeenCalled(); + }); +}); + +describe("message_sending terminal cancel semantics", () => { + let registry: PluginRegistry; + + beforeEach(() => { + registry = createEmptyPluginRegistry(); + }); + + it("keeps cancel=true when a lower-priority hook returns cancel=false", async () => { + addMessageSendingHook(registry, "high", () => ({ cancel: true, content: "guarded" }), 100); + addMessageSendingHook(registry, "low", () => ({ cancel: false, content: "override" }), 10); + + const runner = createHookRunner(registry); + const result = await runner.runMessageSending(messageEvent, messageCtx); + + expect(result?.cancel).toBe(true); + expect(result?.content).toBe("guarded"); + }); + + it("treats explicit cancel=false as no-op when no prior hook canceled", async () => { + addMessageSendingHook(registry, "single", () => ({ cancel: false }), 10); + + const runner = createHookRunner(registry); + const result = await runner.runMessageSending(messageEvent, messageCtx); + + expect(result?.cancel).toBeUndefined(); + }); + + it("treats passive handler output as no-op for prior cancel", async () => { + addMessageSendingHook(registry, "high", () => ({ cancel: true }), 100); + addMessageSendingHook(registry, "passive", () => ({}), 10); + + const runner = createHookRunner(registry); + const result = await runner.runMessageSending(messageEvent, messageCtx); + + expect(result?.cancel).toBe(true); + }); + + it("short-circuits lower-priority hooks after cancel=true", async () => { + const high = vi.fn().mockReturnValue({ cancel: true, content: "guarded" }); + const low = vi.fn().mockReturnValue({ cancel: false, content: "mutated" }); + addMessageSendingHook(registry, "high", high, 100); + addMessageSendingHook(registry, "low", low, 10); + + const runner = createHookRunner(registry); + const result = await runner.runMessageSending(messageEvent, messageCtx); + + expect(result?.cancel).toBe(true); + expect(result?.content).toBe("guarded"); + expect(high).toHaveBeenCalledTimes(1); + expect(low).not.toHaveBeenCalled(); + }); + + it("preserves deterministic same-priority registration order for non-terminal merges", async () => { + addMessageSendingHook(registry, "first", () => ({ content: "first" }), 50); + addMessageSendingHook(registry, "second", () => ({ content: "second" }), 50); + + const runner = createHookRunner(registry); + const result = await runner.runMessageSending(messageEvent, messageCtx); + + expect(result?.content).toBe("second"); + }); + + it("stops before lower-priority throwing hooks when catchErrors is false", async () => { + addMessageSendingHook(registry, "high", () => ({ cancel: true }), 100); + const low = vi.fn().mockImplementation(() => { + throw new Error("should not run"); + }); + addMessageSendingHook(registry, "low", low, 10); + + const runner = createHookRunner(registry, { catchErrors: false }); + const result = await runner.runMessageSending(messageEvent, messageCtx); + + expect(result?.cancel).toBe(true); + expect(low).not.toHaveBeenCalled(); + }); + + it("allows lower-priority cancel when higher-priority hooks are non-terminal", async () => { + addMessageSendingHook(registry, "high-passive", () => ({ content: "rewritten" }), 100); + addMessageSendingHook(registry, "low-cancel", () => ({ cancel: true }), 10); + + const runner = createHookRunner(registry); + const result = await runner.runMessageSending(messageEvent, messageCtx); + + expect(result?.cancel).toBe(true); + }); +}); diff --git a/src/plugins/hooks.ts b/src/plugins/hooks.ts index e8e1e2aa163..c6d97200a0b 100644 --- a/src/plugins/hooks.ts +++ b/src/plugins/hooks.ts @@ -114,6 +114,13 @@ export type HookRunnerOptions = { catchErrors?: boolean; }; +type ModifyingHookPolicy = { + mergeResults?: (accumulated: TResult | undefined, next: TResult) => TResult; + shouldStop?: (result: TResult) => boolean; + terminalLabel?: string; + onTerminal?: (params: { hookName: K; pluginId: string; result: TResult }) => void; +}; + export type PluginTargetedInboundClaimOutcome = | { status: "handled"; @@ -160,20 +167,25 @@ export function createHookRunner(registry: PluginRegistry, options: HookRunnerOp const logger = options.logger; const catchErrors = options.catchErrors ?? true; + const firstDefined = (prev: T | undefined, next: T | undefined): T | undefined => prev ?? next; + const lastDefined = (prev: T | undefined, next: T | undefined): T | undefined => next ?? prev; + const stickyTrue = (prev?: boolean, next?: boolean): true | undefined => + prev === true || next === true ? true : undefined; + const mergeBeforeModelResolve = ( acc: PluginHookBeforeModelResolveResult | undefined, next: PluginHookBeforeModelResolveResult, ): PluginHookBeforeModelResolveResult => ({ // Keep the first defined override so higher-priority hooks win. - modelOverride: acc?.modelOverride ?? next.modelOverride, - providerOverride: acc?.providerOverride ?? next.providerOverride, + modelOverride: firstDefined(acc?.modelOverride, next.modelOverride), + providerOverride: firstDefined(acc?.providerOverride, next.providerOverride), }); const mergeBeforePromptBuild = ( acc: PluginHookBeforePromptBuildResult | undefined, next: PluginHookBeforePromptBuildResult, ): PluginHookBeforePromptBuildResult => ({ - systemPrompt: next.systemPrompt ?? acc?.systemPrompt, + systemPrompt: lastDefined(acc?.systemPrompt, next.systemPrompt), prependContext: concatOptionalTextSegments({ left: acc?.prependContext, right: next.prependContext, @@ -270,7 +282,7 @@ export function createHookRunner(registry: PluginRegistry, options: HookRunnerOp hookName: K, event: Parameters["handler"]>>[0], ctx: Parameters["handler"]>>[1], - mergeResults?: (accumulated: TResult | undefined, next: TResult) => TResult, + policy: ModifyingHookPolicy = {}, ): Promise { const hooks = getHooksForName(registry, hookName); if (hooks.length === 0) { @@ -288,11 +300,20 @@ export function createHookRunner(registry: PluginRegistry, options: HookRunnerOp )(event, ctx); if (handlerResult !== undefined && handlerResult !== null) { - if (mergeResults && result !== undefined) { - result = mergeResults(result, handlerResult); + if (policy.mergeResults) { + result = policy.mergeResults(result, handlerResult); } else { result = handlerResult; } + if (result && policy.shouldStop?.(result)) { + const terminalLabel = policy.terminalLabel ? ` ${policy.terminalLabel}` : ""; + const priority = hook.priority ?? 0; + logger?.debug?.( + `[hooks] ${hookName}${terminalLabel} decided by ${hook.pluginId} (priority=${priority}); skipping remaining handlers`, + ); + policy.onTerminal?.({ hookName, pluginId: hook.pluginId, result }); + break; + } } } catch (err) { handleHookError({ hookName, pluginId: hook.pluginId, error: err }); @@ -434,7 +455,7 @@ export function createHookRunner(registry: PluginRegistry, options: HookRunnerOp "before_model_resolve", event, ctx, - mergeBeforeModelResolve, + { mergeResults: mergeBeforeModelResolve }, ); } @@ -450,7 +471,7 @@ export function createHookRunner(registry: PluginRegistry, options: HookRunnerOp "before_prompt_build", event, ctx, - mergeBeforePromptBuild, + { mergeResults: mergeBeforePromptBuild }, ); } @@ -466,10 +487,12 @@ export function createHookRunner(registry: PluginRegistry, options: HookRunnerOp "before_agent_start", event, ctx, - (acc, next) => ({ - ...mergeBeforePromptBuild(acc, next), - ...mergeBeforeModelResolve(acc, next), - }), + { + mergeResults: (acc, next) => ({ + ...mergeBeforePromptBuild(acc, next), + ...mergeBeforeModelResolve(acc, next), + }), + }, ); } @@ -604,10 +627,19 @@ export function createHookRunner(registry: PluginRegistry, options: HookRunnerOp "message_sending", event, ctx, - (acc, next) => ({ - content: next.content ?? acc?.content, - cancel: next.cancel ?? acc?.cancel, - }), + { + mergeResults: (acc, next) => { + if (acc?.cancel === true) { + return acc; + } + return { + content: lastDefined(acc?.content, next.content), + cancel: stickyTrue(acc?.cancel, next.cancel), + }; + }, + shouldStop: (result) => result.cancel === true, + terminalLabel: "cancel=true", + }, ); } @@ -639,11 +671,20 @@ export function createHookRunner(registry: PluginRegistry, options: HookRunnerOp "before_tool_call", event, ctx, - (acc, next) => ({ - params: next.params ?? acc?.params, - block: next.block ?? acc?.block, - blockReason: next.blockReason ?? acc?.blockReason, - }), + { + mergeResults: (acc, next) => { + if (acc?.block === true) { + return acc; + } + return { + params: lastDefined(acc?.params, next.params), + block: stickyTrue(acc?.block, next.block), + blockReason: lastDefined(acc?.blockReason, next.blockReason), + }; + }, + shouldStop: (result) => result.block === true, + terminalLabel: "block=true", + }, ); } @@ -832,7 +873,7 @@ export function createHookRunner(registry: PluginRegistry, options: HookRunnerOp "subagent_spawning", event, ctx, - mergeSubagentSpawningResult, + { mergeResults: mergeSubagentSpawningResult }, ); } @@ -848,7 +889,7 @@ export function createHookRunner(registry: PluginRegistry, options: HookRunnerOp "subagent_delivery_target", event, ctx, - mergeSubagentDeliveryTargetResult, + { mergeResults: mergeSubagentDeliveryTargetResult }, ); }