Skip to content

Commit e593122

Browse files
vincentkocvincentkoc
committed
fix(hooks): standardize outbound routing metadata
1 parent b0f6c54 commit e593122

12 files changed

Lines changed: 174 additions & 113 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ Docs: https://docs.openclaw.ai
1919

2020
### Fixes
2121

22+
- Hooks/Slack: standardize shared message hook routing fields (`threadId` / `replyToId`) and stop Slack outbound delivery from re-running `message_sending` inside the channel adapter, so plugins like thread-ownership make one outbound routing decision per reply. Thanks @vincentkoc.
2223
- Gateway/restart: preserve group and channel chat context when resuming an agent turn after a Gateway restart, so continuation replies keep the same prompt, routing, and tool-status behavior as the original conversation.
2324
- Gateway/pairing: shared-secret loopback CLI clients now silently auto-approve `metadata-upgrade` pairing (platform / device family refresh) instead of being disconnected with `1008 pairing required`. This matches the scope-upgrade and role-upgrade behavior added in #69431 and unblocks non-interactive CLI automation when a paired-device record has a stale platform string (e.g. device key replicated across hosts, install migrated between OSes, or platform-string format changed between OpenClaw versions). Browser / Control-UI clients keep the existing approval-required flow for metadata changes.
2425
- Gateway/pairing: treat any forwarded-header evidence (`Forwarded`, `X-Forwarded-*`, or `X-Real-IP`) as proxied WebSocket traffic before pairing locality checks, so reverse-proxy topologies cannot use the loopback shared-secret helper auto-pairing path.

‎docs/plugins/building-plugins.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,8 @@ Hook guard semantics to keep in mind:
190190
- `before_install`: `{ block: false }` is treated as no decision.
191191
- `message_sending`: `{ cancel: true }` is terminal and stops lower-priority handlers.
192192
- `message_sending`: `{ cancel: false }` is treated as no decision.
193+
- `message_received`: prefer the typed `threadId` field when you need inbound thread/topic routing. Keep `metadata` for channel-specific extras.
194+
- `message_sending`: prefer typed `replyToId` / `threadId` routing fields over channel-specific metadata keys.
193195

194196
The `/approve` command handles both exec and plugin approvals with bounded fallback: when an exec approval id is not found, OpenClaw retries the same id through plugin approvals. Plugin approval forwarding can be configured independently via `approvals.plugin` in config.
195197

‎docs/plugins/sdk-overview.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,8 @@ AI CLI backend such as `codex-cli`.
460460
- `reply_dispatch`: returning `{ handled: true, ... }` is terminal. Once any handler claims dispatch, lower-priority handlers and the default model dispatch path are skipped.
461461
- `message_sending`: returning `{ cancel: true }` is terminal. Once any handler sets it, lower-priority handlers are skipped.
462462
- `message_sending`: returning `{ cancel: false }` is treated as no decision (same as omitting `cancel`), not as an override.
463+
- `message_received`: use the typed `threadId` field when you need inbound thread/topic routing. Keep `metadata` for channel-specific extras.
464+
- `message_sending`: use typed `replyToId` / `threadId` routing fields before falling back to channel-specific `metadata`.
463465

464466
### API object fields
465467

‎extensions/slack/src/outbound-adapter.test.ts‎

Lines changed: 0 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,11 @@
11
import { beforeEach, describe, expect, it, vi } from "vitest";
22

33
const sendMessageSlackMock = vi.hoisted(() => vi.fn());
4-
const hasHooksMock = vi.hoisted(() => vi.fn());
5-
const runMessageSendingMock = vi.hoisted(() => vi.fn());
64

75
vi.mock("./send.js", () => ({
86
sendMessageSlack: (...args: unknown[]) => sendMessageSlackMock(...args),
97
}));
108

11-
vi.mock("openclaw/plugin-sdk/plugin-runtime", () => ({
12-
getGlobalHookRunner: () => ({
13-
hasHooks: (...args: unknown[]) => hasHooksMock(...args),
14-
runMessageSending: (...args: unknown[]) => runMessageSendingMock(...args),
15-
}),
16-
}));
17-
189
let slackOutbound: typeof import("./outbound-adapter.js").slackOutbound;
1910
({ slackOutbound } = await import("./outbound-adapter.js"));
2011

@@ -30,9 +21,6 @@ describe("slackOutbound", () => {
3021

3122
beforeEach(() => {
3223
sendMessageSlackMock.mockReset();
33-
hasHooksMock.mockReset();
34-
runMessageSendingMock.mockReset();
35-
hasHooksMock.mockReturnValue(false);
3624
});
3725

3826
it("sends payload media first, then finalizes with blocks", async () => {
@@ -127,37 +115,4 @@ describe("slackOutbound", () => {
127115
);
128116
expect(result).toEqual({ channel: "slack", messageId: "m-blocks" });
129117
});
130-
131-
it("cancels sendMedia when message_sending hooks block it", async () => {
132-
hasHooksMock.mockReturnValue(true);
133-
runMessageSendingMock.mockResolvedValue({ cancel: true });
134-
135-
const result = await slackOutbound.sendMedia!({
136-
cfg,
137-
to: "C123",
138-
text: "caption",
139-
mediaUrl: "https://example.com/image.png",
140-
accountId: "default",
141-
replyToId: "1712000000.000001",
142-
});
143-
144-
expect(runMessageSendingMock).toHaveBeenCalledWith(
145-
{
146-
to: "C123",
147-
content: "caption",
148-
metadata: {
149-
threadTs: "1712000000.000001",
150-
channelId: "C123",
151-
mediaUrl: "https://example.com/image.png",
152-
},
153-
},
154-
{ channelId: "slack", accountId: "default" },
155-
);
156-
expect(sendMessageSlackMock).not.toHaveBeenCalled();
157-
expect(result).toMatchObject({
158-
channel: "slack",
159-
messageId: "cancelled-by-hook",
160-
meta: { cancelled: true },
161-
});
162-
});
163118
});

‎extensions/slack/src/outbound-adapter.ts‎

Lines changed: 2 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,12 @@ import {
1212
resolveOutboundSendDep,
1313
type OutboundIdentity,
1414
} from "openclaw/plugin-sdk/outbound-runtime";
15-
import { getGlobalHookRunner } from "openclaw/plugin-sdk/plugin-runtime";
1615
import {
1716
resolvePayloadMediaUrls,
1817
sendPayloadMediaSequenceAndFinalize,
1918
sendTextMediaPayload,
2019
} from "openclaw/plugin-sdk/reply-payload";
2120
import { normalizeOptionalString } from "openclaw/plugin-sdk/text-runtime";
22-
import { resolveSlackAccount } from "./accounts.js";
2321
import { parseSlackBlocksInput } from "./blocks-input.js";
2422
import {
2523
buildSlackInteractiveBlocks,
@@ -64,40 +62,6 @@ function resolveSlackSendIdentity(identity?: OutboundIdentity): SlackSendIdentit
6462
return { username, iconUrl, iconEmoji };
6563
}
6664

67-
async function applySlackMessageSendingHooks(params: {
68-
cfg: NonNullable<NonNullable<Parameters<SlackSendFn>[2]>["cfg"]>;
69-
to: string;
70-
text: string;
71-
threadTs?: string;
72-
accountId?: string;
73-
mediaUrl?: string;
74-
}): Promise<{ cancelled: boolean; text: string }> {
75-
const hookRunner = getGlobalHookRunner();
76-
if (!hookRunner?.hasHooks("message_sending")) {
77-
return { cancelled: false, text: params.text };
78-
}
79-
const account = resolveSlackAccount({
80-
cfg: params.cfg,
81-
accountId: params.accountId,
82-
});
83-
const hookResult = await hookRunner.runMessageSending(
84-
{
85-
to: params.to,
86-
content: params.text,
87-
metadata: {
88-
threadTs: params.threadTs,
89-
channelId: params.to,
90-
...(params.mediaUrl ? { mediaUrl: params.mediaUrl } : {}),
91-
},
92-
},
93-
{ channelId: "slack", accountId: account.accountId },
94-
);
95-
if (hookResult?.cancel) {
96-
return { cancelled: true, text: params.text };
97-
}
98-
return { cancelled: false, text: hookResult?.content ?? params.text };
99-
}
100-
10165
async function sendSlackOutboundMessage(params: {
10266
cfg: NonNullable<NonNullable<Parameters<SlackSendFn>[2]>["cfg"]>;
10367
to: string;
@@ -119,28 +83,10 @@ async function sendSlackOutboundMessage(params: {
11983
const send =
12084
resolveOutboundSendDep<SlackSendFn>(params.deps, "slack") ??
12185
(await loadSlackSendRuntime()).sendMessageSlack;
122-
const threadTs =
123-
params.replyToId ?? (params.threadId != null ? String(params.threadId) : undefined);
124-
const hookResult = await applySlackMessageSendingHooks({
125-
cfg: params.cfg,
126-
to: params.to,
127-
text: params.text,
128-
threadTs,
129-
mediaUrl: params.mediaUrl,
130-
accountId: params.accountId ?? undefined,
131-
});
132-
if (hookResult.cancelled) {
133-
return {
134-
messageId: "cancelled-by-hook",
135-
channelId: params.to,
136-
meta: { cancelled: true },
137-
};
138-
}
139-
14086
const slackIdentity = resolveSlackSendIdentity(params.identity);
141-
const result = await send(params.to, hookResult.text, {
87+
const result = await send(params.to, params.text, {
14288
cfg: params.cfg,
143-
threadTs,
89+
threadTs: params.replyToId ?? (params.threadId != null ? String(params.threadId) : undefined),
14490
accountId: params.accountId ?? undefined,
14591
...(params.mediaUrl
14692
? {

‎extensions/slack/src/outbound-delivery.test.ts‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
2+
import { deliverOutboundPayloads } from "../../../src/infra/outbound/deliver.js";
3+
import {
4+
initializeGlobalHookRunner,
5+
resetGlobalHookRunner,
6+
} from "../../../src/plugins/hook-runner-global.js";
7+
import { addTestHook } from "../../../src/plugins/hooks.test-helpers.js";
8+
import { createEmptyPluginRegistry } from "../../../src/plugins/registry.js";
9+
import {
10+
releasePinnedPluginChannelRegistry,
11+
setActivePluginRegistry,
12+
} from "../../../src/plugins/runtime.js";
13+
import type { PluginHookRegistration } from "../../../src/plugins/types.js";
14+
import {
15+
createOutboundTestPlugin,
16+
createTestRegistry,
17+
} from "../../../src/test-utils/channel-plugins.js";
18+
import { slackOutbound } from "./outbound-adapter.js";
19+
import type { OpenClawConfig } from "./runtime-api.js";
20+
21+
const sendMessageSlackMock = vi.hoisted(() => vi.fn());
22+
23+
vi.mock("./send.runtime.js", () => ({
24+
sendMessageSlack: sendMessageSlackMock,
25+
}));
26+
27+
const cfg: OpenClawConfig = {
28+
channels: {
29+
slack: {
30+
botToken: "xoxb-test",
31+
appToken: "xapp-test",
32+
accounts: {
33+
default: {
34+
botToken: "xoxb-default",
35+
appToken: "xapp-default",
36+
},
37+
},
38+
},
39+
},
40+
};
41+
42+
describe("slack outbound shared hook wiring", () => {
43+
beforeEach(() => {
44+
sendMessageSlackMock.mockReset();
45+
sendMessageSlackMock.mockResolvedValue({ messageId: "m1", channelId: "C123" });
46+
setActivePluginRegistry(
47+
createTestRegistry([
48+
{
49+
pluginId: "slack",
50+
plugin: createOutboundTestPlugin({ id: "slack", outbound: slackOutbound }),
51+
source: "test",
52+
},
53+
]),
54+
);
55+
resetGlobalHookRunner();
56+
});
57+
58+
afterEach(() => {
59+
resetGlobalHookRunner();
60+
releasePinnedPluginChannelRegistry();
61+
});
62+
63+
it("fires message_sending once with shared routing fields", async () => {
64+
const hookRegistry = createEmptyPluginRegistry();
65+
const handler = vi.fn().mockResolvedValue(undefined);
66+
addTestHook({
67+
registry: hookRegistry,
68+
pluginId: "thread-ownership",
69+
hookName: "message_sending",
70+
handler: handler as PluginHookRegistration["handler"],
71+
});
72+
initializeGlobalHookRunner(hookRegistry);
73+
74+
await deliverOutboundPayloads({
75+
cfg,
76+
channel: "slack",
77+
to: "C123",
78+
payloads: [{ text: "hello" }],
79+
accountId: "default",
80+
replyToId: "1712000000.000001",
81+
});
82+
83+
expect(handler).toHaveBeenCalledTimes(1);
84+
expect(handler).toHaveBeenCalledWith(
85+
expect.objectContaining({
86+
to: "C123",
87+
content: "hello",
88+
replyToId: "1712000000.000001",
89+
}),
90+
expect.objectContaining({
91+
channelId: "slack",
92+
accountId: "default",
93+
conversationId: "C123",
94+
}),
95+
);
96+
expect(sendMessageSlackMock).toHaveBeenCalledTimes(1);
97+
});
98+
99+
it("respects cancel from the shared hook without a second adapter pass", async () => {
100+
const hookRegistry = createEmptyPluginRegistry();
101+
const handler = vi.fn().mockResolvedValue({ cancel: true });
102+
addTestHook({
103+
registry: hookRegistry,
104+
pluginId: "thread-ownership",
105+
hookName: "message_sending",
106+
handler: handler as PluginHookRegistration["handler"],
107+
});
108+
initializeGlobalHookRunner(hookRegistry);
109+
110+
const result = await deliverOutboundPayloads({
111+
cfg,
112+
channel: "slack",
113+
to: "C123",
114+
payloads: [{ text: "hello" }],
115+
accountId: "default",
116+
replyToId: "1712000000.000001",
117+
});
118+
119+
expect(handler).toHaveBeenCalledTimes(1);
120+
expect(sendMessageSlackMock).not.toHaveBeenCalled();
121+
expect(result).toEqual([]);
122+
});
123+
});

‎extensions/telegram/src/bot/delivery.replies.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -676,11 +676,15 @@ export async function deliverReplies(params: {
676676
}
677677

678678
const rawContent = reply.text || "";
679+
const replyToId =
680+
params.replyToMode === "off" ? undefined : resolveTelegramReplyId(reply.replyToId);
679681
if (hasMessageSendingHooks) {
680682
const hookResult = await hookRunner?.runMessageSending(
681683
{
682684
to: params.chatId,
683685
content: rawContent,
686+
replyToId,
687+
threadId: params.thread?.id,
684688
metadata: {
685689
channel: "telegram",
686690
mediaUrls: mediaList,
@@ -705,8 +709,6 @@ export async function deliverReplies(params: {
705709

706710
try {
707711
const deliveredCountBeforeReply = progress.deliveredCount;
708-
const replyToId =
709-
params.replyToMode === "off" ? undefined : resolveTelegramReplyId(reply.replyToId);
710712
const telegramData = reply.channelData?.telegram as TelegramReplyChannelData | undefined;
711713
const replyMarkup = buildInlineKeyboard(telegramData?.buttons);
712714
let firstDeliveredMessageId: number | undefined;

0 commit comments

Comments
 (0)