From 9c970561daf16ecd4d96c3518adce4197566628c Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Mon, 9 Mar 2026 12:12:02 -0700 Subject: [PATCH] fix(hooks): deliver internal hook replies on replyable surfaces --- docs/automation/hooks.md | 16 ++++- src/auto-reply/reply/commands-core.test.ts | 61 ++++++++++++++++++ src/auto-reply/reply/commands-core.ts | 38 +++++------ .../reply/dispatch-from-config.test.ts | 55 ++++++++++++++++ src/auto-reply/reply/dispatch-from-config.ts | 36 ++++++++--- src/auto-reply/reply/internal-hook-replies.ts | 64 +++++++++++++++++++ src/hooks/internal-hooks.ts | 2 +- 7 files changed, 237 insertions(+), 35 deletions(-) create mode 100644 src/auto-reply/reply/internal-hook-replies.ts diff --git a/docs/automation/hooks.md b/docs/automation/hooks.md index deda79d3db5..c3c2ad536d3 100644 --- a/docs/automation/hooks.md +++ b/docs/automation/hooks.md @@ -199,7 +199,7 @@ const myHandler = async (event) => { // Your custom logic here - // Optionally send message to user + // Optionally send a reply on supported reply-capable surfaces event.messages.push("✨ My hook executed!"); }; @@ -216,7 +216,7 @@ Each event includes: action: string, // e.g., 'new', 'reset', 'stop', 'received', 'sent' sessionKey: string, // Session identifier timestamp: Date, // When the event occurred - messages: string[], // Push messages here to send to user + messages: string[], // Push reply messages here for supported reply-capable surfaces context: { // Command events: sessionEntry?: SessionEntry, @@ -339,6 +339,18 @@ Message events include rich context about the message: } ``` +#### Hook Reply Messages + +`event.messages` is not a global "reply anywhere" mechanism. + +OpenClaw only drains `event.messages` on reply-capable surfaces where it has a safe routing target: + +- `command:new` +- `command:reset` +- `message:received` + +For lifecycle-only surfaces such as `agent:bootstrap`, `message:preprocessed`, `message:transcribed`, `message:sent`, and gateway/session events, pushed `event.messages` are not automatically delivered to the user. + #### Example: Message Logger Hook ```typescript diff --git a/src/auto-reply/reply/commands-core.test.ts b/src/auto-reply/reply/commands-core.test.ts index 226037f957a..a40d1f91484 100644 --- a/src/auto-reply/reply/commands-core.test.ts +++ b/src/auto-reply/reply/commands-core.test.ts @@ -1,4 +1,9 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { + clearInternalHooks, + registerInternalHook, + unregisterInternalHook, +} from "../../hooks/internal-hooks.js"; import type { HookRunner } from "../../plugins/hooks.js"; import type { HandleCommandsParams } from "./commands-types.js"; @@ -6,6 +11,9 @@ const hookRunnerMocks = vi.hoisted(() => ({ hasHooks: vi.fn(), runBeforeReset: vi.fn(), })); +const routeReplyMocks = vi.hoisted(() => ({ + routeReply: vi.fn(async () => ({ ok: true, messageId: "hook-reply" })), +})); vi.mock("../../plugins/hook-runner-global.js", () => ({ getGlobalHookRunner: () => @@ -14,6 +22,13 @@ vi.mock("../../plugins/hook-runner-global.js", () => ({ runBeforeReset: hookRunnerMocks.runBeforeReset, }) as unknown as HookRunner, })); +vi.mock("./route-reply.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + routeReply: routeReplyMocks.routeReply, + }; +}); const { emitResetCommandHooks } = await import("./commands-core.js"); @@ -46,13 +61,17 @@ describe("emitResetCommandHooks", () => { } beforeEach(() => { + clearInternalHooks(); hookRunnerMocks.hasHooks.mockReset(); hookRunnerMocks.runBeforeReset.mockReset(); hookRunnerMocks.hasHooks.mockImplementation((hookName) => hookName === "before_reset"); hookRunnerMocks.runBeforeReset.mockResolvedValue(undefined); + routeReplyMocks.routeReply.mockReset(); + routeReplyMocks.routeReply.mockResolvedValue({ ok: true, messageId: "hook-reply" }); }); afterEach(() => { + clearInternalHooks(); vi.restoreAllMocks(); }); @@ -85,4 +104,46 @@ describe("emitResetCommandHooks", () => { workspaceDir: "/tmp/openclaw-workspace", }); }); + + it("routes hook reply messages for reset/new command hooks", async () => { + const handler = vi.fn((event) => { + event.messages.push("Hook reply"); + }); + registerInternalHook("command:new", handler); + + const command = { + surface: "discord", + senderId: "rai", + channel: "discord", + from: "discord:rai", + to: "discord:bot", + resetHookTriggered: false, + } as HandleCommandsParams["command"]; + + await emitResetCommandHooks({ + action: "new", + ctx: { + AccountId: "acc-1", + MessageThreadId: "thread-1", + } as HandleCommandsParams["ctx"], + cfg: {} as HandleCommandsParams["cfg"], + command, + sessionKey: "agent:main:main", + workspaceDir: "/tmp/openclaw-workspace", + }); + + expect(handler).toHaveBeenCalledOnce(); + expect(routeReplyMocks.routeReply).toHaveBeenCalledWith( + expect.objectContaining({ + payload: { text: "Hook reply" }, + channel: "discord", + to: "discord:rai", + sessionKey: "agent:main:main", + accountId: "acc-1", + threadId: "thread-1", + }), + ); + + unregisterInternalHook("command:new", handler); + }); }); diff --git a/src/auto-reply/reply/commands-core.ts b/src/auto-reply/reply/commands-core.ts index 894724bcfb0..60da6c0b692 100644 --- a/src/auto-reply/reply/commands-core.ts +++ b/src/auto-reply/reply/commands-core.ts @@ -39,7 +39,7 @@ import type { CommandHandlerResult, HandleCommandsParams, } from "./commands-types.js"; -import { routeReply } from "./route-reply.js"; +import { deliverInternalHookMessages } from "./internal-hook-replies.js"; let HANDLERS: CommandHandler[] | null = null; @@ -69,27 +69,21 @@ export async function emitResetCommandHooks(params: { await triggerInternalHook(hookEvent); params.command.resetHookTriggered = true; - // Send hook messages immediately if present - if (hookEvent.messages.length > 0) { - // Use OriginatingChannel/To if available, otherwise fall back to command channel/from - // oxlint-disable-next-line typescript/no-explicit-any - const channel = params.ctx.OriginatingChannel || (params.command.channel as any); - // For replies, use 'from' (the sender) not 'to' (which might be the bot itself) - const to = params.ctx.OriginatingTo || params.command.from || params.command.to; - - if (channel && to) { - const hookReply = { text: hookEvent.messages.join("\n\n") }; - await routeReply({ - payload: hookReply, - channel: channel, - to: to, - sessionKey: params.sessionKey, - accountId: params.ctx.AccountId, - threadId: params.ctx.MessageThreadId, - cfg: params.cfg, - }); - } - } + // /new and /reset are interactive surfaces, so hook replies can be sent immediately. + await deliverInternalHookMessages({ + event: hookEvent, + target: { + cfg: params.cfg, + // oxlint-disable-next-line typescript/no-explicit-any + channel: params.ctx.OriginatingChannel || (params.command.channel as any), + // Use 'from' for command replies because 'to' may be the bot identity. + to: params.ctx.OriginatingTo || params.command.from || params.command.to, + sessionKey: params.sessionKey, + accountId: params.ctx.AccountId, + threadId: params.ctx.MessageThreadId, + }, + source: "emitResetCommandHooks", + }); // Fire before_reset plugin hook — extract memories before session history is lost const hookRunner = getGlobalHookRunner(); diff --git a/src/auto-reply/reply/dispatch-from-config.test.ts b/src/auto-reply/reply/dispatch-from-config.test.ts index 982557ecb68..e4092996f7d 100644 --- a/src/auto-reply/reply/dispatch-from-config.test.ts +++ b/src/auto-reply/reply/dispatch-from-config.test.ts @@ -1658,6 +1658,61 @@ describe("dispatchReplyFromConfig", () => { expect(internalHookMocks.triggerInternalHook).toHaveBeenCalledTimes(1); }); + it("routes internal message:received hook replies on replyable surfaces", async () => { + setNoAbort(); + const cfg = emptyConfig; + const dispatcher = createDispatcher(); + const ctx = buildTestCtx({ + Provider: "whatsapp", + Surface: "whatsapp", + SessionKey: "agent:main:main", + To: "whatsapp:+2000", + From: "whatsapp:+1000", + CommandBody: "hello", + }); + internalHookMocks.triggerInternalHook.mockImplementation(async (...args: unknown[]) => { + const [event] = args as [{ messages: string[] }]; + event.messages.push("Hook reply"); + }); + + const replyResolver = async () => ({ text: "agent reply" }) satisfies ReplyPayload; + await dispatchReplyFromConfig({ ctx, cfg, dispatcher, replyResolver }); + + await vi.waitFor(() => + expect(mocks.routeReply).toHaveBeenCalledWith( + expect.objectContaining({ + payload: { text: "Hook reply" }, + channel: "whatsapp", + to: "whatsapp:+2000", + sessionKey: "agent:main:main", + }), + ), + ); + }); + + it("does not route internal hook replies from non-routable surfaces", async () => { + setNoAbort(); + const cfg = emptyConfig; + const dispatcher = createDispatcher(); + const ctx = buildTestCtx({ + Provider: "webchat", + Surface: "webchat", + SessionKey: "agent:main:main", + To: "session:abc", + }); + internalHookMocks.triggerInternalHook.mockImplementation(async (...args: unknown[]) => { + const [event] = args as [{ messages: string[] }]; + event.messages.push("Hook reply"); + }); + + const replyResolver = async () => ({ text: "agent reply" }) satisfies ReplyPayload; + await dispatchReplyFromConfig({ ctx, cfg, dispatcher, replyResolver }); + + await vi.waitFor(() => expect(internalHookMocks.triggerInternalHook).toHaveBeenCalledTimes(1)); + await Promise.resolve(); + expect(mocks.routeReply).not.toHaveBeenCalled(); + }); + it("skips internal message:received hook when session key is unavailable", async () => { setNoAbort(); const cfg = emptyConfig; diff --git a/src/auto-reply/reply/dispatch-from-config.ts b/src/auto-reply/reply/dispatch-from-config.ts index 786b1a7c16b..c18e37bb6d7 100644 --- a/src/auto-reply/reply/dispatch-from-config.ts +++ b/src/auto-reply/reply/dispatch-from-config.ts @@ -31,6 +31,7 @@ import type { GetReplyOptions, ReplyPayload } from "../types.js"; import { formatAbortReplyText, tryFastAbortFromMessage } from "./abort.js"; import { shouldBypassAcpDispatchForCommand, tryDispatchAcpReply } from "./dispatch-acp.js"; import { shouldSkipDuplicateInbound } from "./inbound-dedupe.js"; +import { deliverInternalHookMessages } from "./internal-hook-replies.js"; import type { ReplyDispatcher, ReplyDispatchKind } from "./reply-dispatcher.js"; import { shouldSuppressReasoningPayload } from "./reply-payloads.js"; import { isRoutableChannel, routeReply } from "./route-reply.js"; @@ -182,6 +183,12 @@ export async function dispatchReplyFromConfig(params: { ctx.MessageSidFull ?? ctx.MessageSid ?? ctx.MessageSidFirst ?? ctx.MessageSidLast; const hookContext = deriveInboundMessageHookContext(ctx, { messageId: messageIdForHook }); const { isGroup, groupId } = hookContext; + const originatingChannel = normalizeMessageChannel(ctx.OriginatingChannel); + const originatingTo = ctx.OriginatingTo; + const providerChannel = normalizeMessageChannel(ctx.Provider); + const surfaceChannel = normalizeMessageChannel(ctx.Surface); + // Prefer provider channel because surface may carry origin metadata in relayed flows. + const currentSurface = providerChannel ?? surfaceChannel; // Trigger plugin hooks (fire-and-forget) if (hookRunner?.hasHooks("message_received")) { @@ -197,12 +204,27 @@ export async function dispatchReplyFromConfig(params: { // Bridge to internal hooks (HOOK.md discovery system) - refs #8807 if (sessionKey) { fireAndForgetHook( - triggerInternalHook( - createInternalHookEvent("message", "received", sessionKey, { + (async () => { + const hookEvent = createInternalHookEvent("message", "received", sessionKey, { ...toInternalMessageReceivedContext(hookContext), timestamp, - }), - ), + }); + await triggerInternalHook(hookEvent); + await deliverInternalHookMessages({ + event: hookEvent, + target: { + cfg, + channel: originatingChannel ?? currentSurface, + to: originatingTo ?? ctx.To ?? ctx.From, + sessionKey, + accountId: ctx.AccountId, + threadId: ctx.MessageThreadId, + isGroup, + groupId, + }, + source: "dispatch-from-config: message_received", + }); + })(), "dispatch-from-config: message_received internal hook failed", ); } @@ -214,12 +236,6 @@ export async function dispatchReplyFromConfig(params: { // flow when the provider handles its own messages. // // Debug: `pnpm test src/auto-reply/reply/dispatch-from-config.test.ts` - const originatingChannel = normalizeMessageChannel(ctx.OriginatingChannel); - const originatingTo = ctx.OriginatingTo; - const providerChannel = normalizeMessageChannel(ctx.Provider); - const surfaceChannel = normalizeMessageChannel(ctx.Surface); - // Prefer provider channel because surface may carry origin metadata in relayed flows. - const currentSurface = providerChannel ?? surfaceChannel; const isInternalWebchatTurn = currentSurface === INTERNAL_MESSAGE_CHANNEL && (surfaceChannel === INTERNAL_MESSAGE_CHANNEL || !surfaceChannel) && diff --git a/src/auto-reply/reply/internal-hook-replies.ts b/src/auto-reply/reply/internal-hook-replies.ts new file mode 100644 index 00000000000..f0dc169e009 --- /dev/null +++ b/src/auto-reply/reply/internal-hook-replies.ts @@ -0,0 +1,64 @@ +import type { OpenClawConfig } from "../../config/config.js"; +import { logVerbose } from "../../globals.js"; +import type { InternalHookEvent } from "../../hooks/internal-hooks.js"; +import { normalizeMessageChannel } from "../../utils/message-channel.js"; +import type { OriginatingChannelType } from "../templating.js"; +import { isRoutableChannel, routeReply } from "./route-reply.js"; + +export type InternalHookReplyTarget = { + cfg: OpenClawConfig; + channel?: string; + to?: string; + sessionKey?: string; + accountId?: string; + threadId?: string | number; + isGroup?: boolean; + groupId?: string; +}; + +export async function deliverInternalHookMessages(params: { + event: InternalHookEvent; + target: InternalHookReplyTarget; + source: string; +}): Promise { + if (params.event.messages.length === 0) { + return; + } + + const messages = params.event.messages.filter((message) => message.trim()); + if (messages.length === 0) { + return; + } + const text = messages.join("\n\n"); + + const channel = normalizeMessageChannel( + params.target.channel as OriginatingChannelType | undefined, + ); + if (!channel || !isRoutableChannel(channel)) { + logVerbose(`${params.source}: hook replies skipped on non-routable surface`); + return; + } + + const to = params.target.to?.trim(); + if (!to) { + logVerbose(`${params.source}: hook replies skipped without a reply target`); + return; + } + + const result = await routeReply({ + payload: { text }, + channel, + to, + sessionKey: params.target.sessionKey, + accountId: params.target.accountId, + threadId: params.target.threadId, + cfg: params.target.cfg, + ...(params.target.isGroup != null ? { isGroup: params.target.isGroup } : {}), + ...(params.target.groupId ? { groupId: params.target.groupId } : {}), + }); + if (!result.ok) { + logVerbose( + `${params.source}: failed to route hook replies: ${result.error ?? "unknown error"}`, + ); + } +} diff --git a/src/hooks/internal-hooks.ts b/src/hooks/internal-hooks.ts index b73dcb75fab..a93c9a82a13 100644 --- a/src/hooks/internal-hooks.ts +++ b/src/hooks/internal-hooks.ts @@ -167,7 +167,7 @@ export interface InternalHookEvent { context: Record; /** Timestamp when the event occurred */ timestamp: Date; - /** Messages to send back to the user (hooks can push to this array) */ + /** Reply messages for supported reply-capable surfaces (hooks can push to this array) */ messages: string[]; }