Skip to content

Commit 7e7fc00

Browse files
scoootscooobsteipete
scoootscooob
and
steipete
authored
[codex] Honor all ack scope for room events (#87433)
* fix(discord): honor all ack scope for room events * fix(channels): align all ack scope for room events * fix(channels): centralize ambient ack scope * test(telegram): restore room-event ack fixture * test(discord): satisfy promise executor lint --------- Co-authored-by: Peter Steinberger <[email protected]>
1 parent 2c31af1 commit 7e7fc00

14 files changed

Lines changed: 217 additions & 16 deletions

‎docs/channels/ambient-room-events.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ With `messages.groupChat.unmentionedInbound: "room_event"`:
4646

4747
Room events use strict visible delivery. Final assistant text is private. The agent must call `message(action=send)` to post in the room.
4848

49+
Typing and lifecycle status reactions stay suppressed for room events. The one explicit receipt exception is `messages.ackReactionScope: "all"`, which sends the configured ack reaction; use any narrower scope or `"off"` when the room must remain completely silent.
50+
4951
## Discord example
5052

5153
```json5

‎docs/channels/discord.md‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -879,6 +879,14 @@ See [Slash commands](/tools/slash-commands) for the command catalog and behavior
879879
- Discord accepts unicode emoji or custom emoji names.
880880
- Use `""` to disable the reaction for a channel or account.
881881

882+
**Scope (`messages.ackReactionScope`):**
883+
884+
Values: `"all"` (DMs + groups, including ambient room events), `"direct"` (DMs only), `"group-all"` (every group message except ambient room events, no DMs), `"group-mentions"` (groups when the bot is mentioned; **no DMs**, default), `"off"` / `"none"` (disabled).
885+
886+
<Note>
887+
The default scope (`"group-mentions"`) does not fire ack reactions in direct messages or ambient room events. To get an ack reaction on inbound Discord DMs and quiet room events, set `messages.ackReactionScope` to `"all"`.
888+
</Note>
889+
882890
</Accordion>
883891

884892
<Accordion title="Config writes">

‎docs/channels/slack.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1102,14 +1102,14 @@ The Slack provider reads scope from `messages.ackReactionScope` (default `"group
11021102

11031103
Values:
11041104

1105-
- `"all"`: react in DMs and groups.
1105+
- `"all"`: react in DMs and groups, including ambient room events.
11061106
- `"direct"`: react in DMs only.
1107-
- `"group-all"`: react on every group message (no DMs).
1107+
- `"group-all"`: react on every group message except ambient room events (no DMs).
11081108
- `"group-mentions"` (default): react in groups, but only when the bot is mentioned (or in group mentionables that opted in). **DMs are excluded.**
11091109
- `"off"` / `"none"`: never react.
11101110

11111111
<Note>
1112-
The default scope (`"group-mentions"`) does not fire ack reactions in direct messages. To see the configured `ackReaction` (for example `"eyes"`) on inbound Slack DMs, set `messages.ackReactionScope` to `"direct"` or `"all"`. `messages.ackReactionScope` is read at Slack provider startup, so a gateway restart is needed for the change to take effect.
1112+
The default scope (`"group-mentions"`) does not fire ack reactions in direct messages or ambient room events. To see the configured `ackReaction` (for example `"eyes"`) on inbound Slack DMs and quiet room events, set `messages.ackReactionScope` to `"all"`. `messages.ackReactionScope` is read at Slack provider startup, so a gateway restart is needed for the change to take effect.
11131113
</Note>
11141114

11151115
```json5

‎docs/channels/telegram.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -667,10 +667,10 @@ curl "https://api.telegram.org/bot<bot_token>/getUpdates"
667667

668668
**Scope (`messages.ackReactionScope`, default `"group-mentions"`; no Telegram-account or Telegram-channel override today):**
669669

670-
`all` (DMs + groups), `direct` (DMs only), `group-all` (every group message, no DMs), `group-mentions` (groups when the bot is mentioned; **no DMs** — default), `off` / `none` (disabled).
670+
`all` (DMs + groups, including ambient room events), `direct` (DMs only), `group-all` (every group message except ambient room events, no DMs), `group-mentions` (groups when the bot is mentioned; **no DMs** — default), `off` / `none` (disabled).
671671

672672
<Note>
673-
The default scope (`group-mentions`) does not fire ack reactions in DMs. Set `messages.ackReactionScope` to `direct` or `all` for that. This value is read at Telegram provider startup, so a gateway restart is needed for the change to take effect.
673+
The default scope (`group-mentions`) does not fire ack reactions in DMs or ambient room events. Use `direct` or `all` for DMs; only `all` acknowledges ambient room events. This value is read at Telegram provider startup, so a gateway restart is needed for the change to take effect.
674674
</Note>
675675

676676
</Accordion>

‎extensions/discord/src/monitor/message-handler.process.test.ts‎

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1618,7 +1618,45 @@ describe("processDiscordMessage session routing", () => {
16181618
expect(emojis).toContain(DEFAULT_EMOJIS.done);
16191619
});
16201620

1621-
it("suppresses Discord reactions for room events even when status reactions are explicit", async () => {
1621+
it("suppresses Discord reactions for room events when ack scope does not force all messages", async () => {
1622+
vi.useFakeTimers();
1623+
dispatchInboundMessage.mockImplementationOnce(async (params?: DispatchInboundParams) => {
1624+
await params?.replyOptions?.onReasoningStream?.();
1625+
await new Promise((resolve) => {
1626+
setTimeout(resolve, 1_000);
1627+
});
1628+
return createNoQueuedDispatchResult();
1629+
});
1630+
const ctx = await createBaseContext({
1631+
shouldRequireMention: false,
1632+
effectiveWasMentioned: false,
1633+
inboundEventKind: "room_event",
1634+
ackReactionScope: "group-all",
1635+
cfg: {
1636+
messages: {
1637+
ackReaction: "👀",
1638+
ackReactionScope: "group-all",
1639+
statusReactions: {
1640+
enabled: true,
1641+
timing: { debounceMs: 0 },
1642+
},
1643+
},
1644+
session: { store: "/tmp/openclaw-discord-process-test-sessions.json" },
1645+
},
1646+
route: BASE_CHANNEL_ROUTE,
1647+
});
1648+
1649+
const runPromise = runProcessDiscordMessage(ctx);
1650+
await vi.advanceTimersByTimeAsync(1_000);
1651+
await vi.runAllTimersAsync();
1652+
await runPromise;
1653+
1654+
expect(getLastDispatchReplyOptions()?.sourceReplyDeliveryMode).toBe("message_tool_only");
1655+
expect(getReactionEmojis()).toEqual([]);
1656+
expect(sendMocks.removeReactionDiscord).not.toHaveBeenCalled();
1657+
});
1658+
1659+
it("sends Discord ack reactions for room events when ack scope is all", async () => {
16221660
vi.useFakeTimers();
16231661
dispatchInboundMessage.mockImplementationOnce(async (params?: DispatchInboundParams) => {
16241662
await params?.replyOptions?.onReasoningStream?.();
@@ -1652,7 +1690,7 @@ describe("processDiscordMessage session routing", () => {
16521690
await runPromise;
16531691

16541692
expect(getLastDispatchReplyOptions()?.sourceReplyDeliveryMode).toBe("message_tool_only");
1655-
expect(getReactionEmojis()).toEqual([]);
1693+
expect(getReactionEmojis()).toEqual(["👀"]);
16561694
expect(sendMocks.removeReactionDiscord).not.toHaveBeenCalled();
16571695
});
16581696

‎extensions/discord/src/monitor/message-handler.process.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,10 +260,10 @@ async function processDiscordMessageInner(
260260
const isRoomEvent = ctx.inboundEventKind === "room_event";
261261
const shouldAckReaction = () =>
262262
Boolean(
263-
!isRoomEvent &&
264263
ackReaction &&
265264
shouldAckReactionGate({
266265
scope: ackReactionScope,
266+
inboundEventKind: ctx.inboundEventKind,
267267
isDirect: isDirectMessage,
268268
isGroup: isGuildMessage || isGroupDm,
269269
isMentionableGroup: isGuildMessage,

‎extensions/slack/src/monitor/message-handler/dispatch.preview-fallback.test.ts‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1812,6 +1812,24 @@ describe("dispatchPreparedSlackMessage preview fallback", () => {
18121812
expect(statusReactionControllerMock.setDone).not.toHaveBeenCalled();
18131813
});
18141814

1815+
it("keeps Slack lifecycle reactions off for ambient room-event acks", async () => {
1816+
await dispatchPreparedSlackMessage(
1817+
createPreparedSlackMessage({
1818+
cfg: { messages: { statusReactions: { enabled: true } } },
1819+
ctxPayload: { ChatType: "channel", InboundEventKind: "room_event" },
1820+
ackReactionMessageTs: "171234.111",
1821+
ackReactionPromise: Promise.resolve(true),
1822+
}),
1823+
);
1824+
1825+
expectRecordFields(requireRecord(capturedStatusReactionOptions, "status reaction options"), {
1826+
enabled: false,
1827+
initialEmoji: "eyes",
1828+
});
1829+
expect(statusReactionControllerMock.setQueued).not.toHaveBeenCalled();
1830+
expect(statusReactionControllerMock.setDone).not.toHaveBeenCalled();
1831+
});
1832+
18151833
it("escapes Slack mrkdwn in tool progress preview labels", async () => {
18161834
const draftStream = createDraftStreamStub();
18171835
createSlackDraftStreamMock.mockReturnValueOnce(draftStream);

‎extensions/slack/src/monitor/message-handler/dispatch.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -512,6 +512,7 @@ export async function dispatchPreparedSlackMessage(prepared: PreparedSlackMessag
512512
const incomingThreadTs = message.thread_ts;
513513
let didSetStatus = false;
514514
const statusReactionsEnabled =
515+
prepared.ctxPayload.InboundEventKind !== "room_event" &&
515516
Boolean(prepared.ackReactionPromise) &&
516517
Boolean(reactionMessageTs) &&
517518
cfg.messages?.statusReactions?.enabled === true;

‎extensions/slack/src/monitor/message-handler/prepare.test.ts‎

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -899,12 +899,12 @@ describe("slack prepareSlackMessage inbound contract", () => {
899899
});
900900
});
901901

902-
it("keeps unmentioned room events quiet even when group replies are automatic", async () => {
902+
it("keeps unmentioned room events quiet when ack scope does not force all messages", async () => {
903903
const slackCtx = createInboundSlackCtx({
904904
cfg: {
905905
messages: {
906906
ackReaction: "eyes",
907-
ackReactionScope: "all",
907+
ackReactionScope: "group-all",
908908
groupChat: {
909909
unmentionedInbound: "room_event",
910910
visibleReplies: "automatic",
@@ -922,6 +922,7 @@ describe("slack prepareSlackMessage inbound contract", () => {
922922
});
923923
slackCtx.resolveUserName = async () => ({ name: "Alice" }) as any;
924924
slackCtx.resolveChannelName = async () => ({ name: "general", type: "channel" });
925+
slackCtx.ackReactionScope = "group-all";
925926

926927
const prepared = await prepareMessageWith(slackCtx, defaultAccount, {
927928
channel: "C123",
@@ -937,6 +938,54 @@ describe("slack prepareSlackMessage inbound contract", () => {
937938
expect(prepared.ackReactionPromise).toBeNull();
938939
});
939940

941+
it("sends Slack ack reactions for room events when ack scope is all", async () => {
942+
const reactionAdd = vi.fn().mockResolvedValue({ ok: true });
943+
const slackCtx = createInboundSlackCtx({
944+
cfg: {
945+
messages: {
946+
ackReaction: "eyes",
947+
ackReactionScope: "all",
948+
groupChat: {
949+
unmentionedInbound: "room_event",
950+
visibleReplies: "automatic",
951+
},
952+
statusReactions: { enabled: true },
953+
},
954+
channels: {
955+
slack: {
956+
enabled: true,
957+
groupPolicy: "open",
958+
},
959+
},
960+
} as OpenClawConfig,
961+
appClient: { reactions: { add: reactionAdd } } as any,
962+
defaultRequireMention: false,
963+
});
964+
slackCtx.resolveUserName = async () => ({ name: "Alice" }) as any;
965+
slackCtx.resolveChannelName = async () => ({ name: "general", type: "channel" });
966+
slackCtx.ackReactionScope = "all";
967+
968+
const prepared = await prepareMessageWith(slackCtx, defaultAccount, {
969+
channel: "C123",
970+
channel_type: "channel",
971+
user: "U1",
972+
text: "ambient note",
973+
ts: "1.000",
974+
} as SlackMessageEvent);
975+
976+
assertPrepared(prepared);
977+
expect(prepared.ctxPayload.InboundEventKind).toBe("room_event");
978+
expect(prepared.ackReactionMessageTs).toBe("1.000");
979+
expect(prepared.ackReactionValue).toBe("eyes");
980+
expect(prepared.ackReactionPromise).toBeInstanceOf(Promise);
981+
expect(await prepared.ackReactionPromise).toBe(true);
982+
expect(reactionAdd).toHaveBeenCalledWith({
983+
channel: "C123",
984+
name: "eyes",
985+
timestamp: "1.000",
986+
});
987+
});
988+
940989
it("keeps unmentioned abort requests as user requests when room events are enabled", async () => {
941990
const slackCtx = createInboundSlackCtx({
942991
cfg: {

‎extensions/slack/src/monitor/message-handler/prepare.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1116,11 +1116,13 @@ export async function prepareSlackMessage(params: {
11161116
ctx: { ChatType: chatType, InboundEventKind: inboundEventKind },
11171117
}) === "message_tool_only";
11181118
const statusReactionsExplicitlyEnabled = cfg.messages?.statusReactions?.enabled === true;
1119+
const isRoomEvent = inboundEventKind === "room_event";
11191120
const shouldAckReaction = () =>
11201121
Boolean(
11211122
ackReaction &&
11221123
shouldAckReactionGate({
11231124
scope: ctx.ackReactionScope as AckReactionScope | undefined,
1125+
inboundEventKind,
11241126
isDirect: isDirectMessage,
11251127
isGroup: isRoomish,
11261128
isMentionableGroup: isRoom,
@@ -1134,10 +1136,13 @@ export async function prepareSlackMessage(params: {
11341136
const ackReactionMessageTs = message.ts;
11351137
const allowToolOnlyStatusReaction =
11361138
statusReactionsExplicitlyEnabled && (effectiveWasMentioned || shouldBypassMention);
1139+
const shouldSendConfiguredAck = shouldAckReaction();
11371140
const shouldSendAckReaction =
1138-
shouldAckReaction() && (!sourceRepliesAreToolOnly || allowToolOnlyStatusReaction);
1141+
shouldSendConfiguredAck &&
1142+
(!sourceRepliesAreToolOnly || allowToolOnlyStatusReaction || isRoomEvent);
11391143
const statusReactionsWillHandle =
11401144
Boolean(ackReactionMessageTs) &&
1145+
!isRoomEvent &&
11411146
statusReactionsExplicitlyEnabled &&
11421147
shouldSendAckReaction;
11431148
const ackReactionPromise =

0 commit comments

Comments
 (0)