mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-12 07:20:45 +00:00
fix(hooks): deliver internal hook replies on replyable surfaces
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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<HookRunner["hasHooks"]>(),
|
||||
runBeforeReset: vi.fn<HookRunner["runBeforeReset"]>(),
|
||||
}));
|
||||
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<typeof import("./route-reply.js")>();
|
||||
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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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) &&
|
||||
|
||||
64
src/auto-reply/reply/internal-hook-replies.ts
Normal file
64
src/auto-reply/reply/internal-hook-replies.ts
Normal file
@@ -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<void> {
|
||||
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"}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -167,7 +167,7 @@ export interface InternalHookEvent {
|
||||
context: Record<string, unknown>;
|
||||
/** 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[];
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user