mirror of
https://github.com/openclaw/openclaw.git
synced 2026-04-19 11:28:38 +00:00
fix: harden slack DM app_mention dedupe and add regressions (#32190) (thanks @zicofernandes)
This commit is contained in:
@@ -128,6 +128,7 @@ Docs: https://docs.openclaw.ai
|
|||||||
- Daemon/systemd checks in containers: treat missing `systemctl` invocations (including `spawn systemctl ENOENT`/`EACCES`) as unavailable service state during `is-enabled` checks, preventing container flows from failing with `Gateway service check failed` before install/status handling can continue. (#26089) Thanks @sahilsatralkar and @vincentkoc.
|
- Daemon/systemd checks in containers: treat missing `systemctl` invocations (including `spawn systemctl ENOENT`/`EACCES`) as unavailable service state during `is-enabled` checks, preventing container flows from failing with `Gateway service check failed` before install/status handling can continue. (#26089) Thanks @sahilsatralkar and @vincentkoc.
|
||||||
- Feishu/Send target prefixes: normalize explicit `group:`/`dm:` send targets and preserve explicit receive-id routing hints when resolving outbound Feishu targets. (#31594) Thanks @liuxiaopai-ai.
|
- Feishu/Send target prefixes: normalize explicit `group:`/`dm:` send targets and preserve explicit receive-id routing hints when resolving outbound Feishu targets. (#31594) Thanks @liuxiaopai-ai.
|
||||||
- Slack/Channel message subscriptions: register explicit `message.channels` and `message.groups` monitor handlers (alongside generic `message`) so channel/group event subscriptions are consumed even when Slack dispatches typed message event names. Fixes #31674.
|
- Slack/Channel message subscriptions: register explicit `message.channels` and `message.groups` monitor handlers (alongside generic `message`) so channel/group event subscriptions are consumed even when Slack dispatches typed message event names. Fixes #31674.
|
||||||
|
- Slack/DM event dedupe: skip `app_mention` delivery for DM channels using normalized channel typing so misreported/missing `channel_type` cannot trigger duplicate DM processing alongside `message` events. (#32190) Thanks @zicofernandes.
|
||||||
- Tests/Sandbox + archive portability: use junction-compatible directory-link setup on Windows and explicit file-symlink platform guards in symlink escape tests where unprivileged file symlinks are unavailable, reducing false Windows CI failures while preserving traversal checks on supported paths. (#28747) Thanks @arosstale.
|
- Tests/Sandbox + archive portability: use junction-compatible directory-link setup on Windows and explicit file-symlink platform guards in symlink escape tests where unprivileged file symlinks are unavailable, reducing false Windows CI failures while preserving traversal checks on supported paths. (#28747) Thanks @arosstale.
|
||||||
- Tests/Subagent announce: set `OPENCLAW_TEST_FAST=1` before importing `subagent-announce` format suites so module-level fast-mode constants are captured deterministically on Windows CI, preventing timeout flakes in nested completion announce coverage. (#31370) Thanks @zwffff.
|
- Tests/Subagent announce: set `OPENCLAW_TEST_FAST=1` before importing `subagent-announce` format suites so module-level fast-mode constants are captured deterministically on Windows CI, preventing timeout flakes in nested completion announce coverage. (#31370) Thanks @zwffff.
|
||||||
|
|
||||||
|
|||||||
@@ -17,6 +17,7 @@ vi.mock("../../../pairing/pairing-store.js", () => ({
|
|||||||
}));
|
}));
|
||||||
|
|
||||||
type MessageHandler = (args: { event: Record<string, unknown>; body: unknown }) => Promise<void>;
|
type MessageHandler = (args: { event: Record<string, unknown>; body: unknown }) => Promise<void>;
|
||||||
|
type MentionHandler = (args: { event: Record<string, unknown>; body: unknown }) => Promise<void>;
|
||||||
|
|
||||||
type MessageCase = {
|
type MessageCase = {
|
||||||
overrides?: SlackSystemEventTestOverrides;
|
overrides?: SlackSystemEventTestOverrides;
|
||||||
@@ -33,6 +34,7 @@ function createMessageHandlers(overrides?: SlackSystemEventTestOverrides) {
|
|||||||
});
|
});
|
||||||
return {
|
return {
|
||||||
handler: harness.getHandler("message") as MessageHandler | null,
|
handler: harness.getHandler("message") as MessageHandler | null,
|
||||||
|
mentionHandler: harness.getHandler("app_mention") as MentionHandler | null,
|
||||||
handleSlackMessage,
|
handleSlackMessage,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
@@ -214,4 +216,73 @@ describe("registerSlackMessageEvents", () => {
|
|||||||
expect(handleSlackMessage).not.toHaveBeenCalled();
|
expect(handleSlackMessage).not.toHaveBeenCalled();
|
||||||
expect(messageQueueMock).toHaveBeenCalledTimes(1);
|
expect(messageQueueMock).toHaveBeenCalledTimes(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("skips app_mention events for D-prefixed DMs even with missing or wrong channel_type", async () => {
|
||||||
|
messageQueueMock.mockClear();
|
||||||
|
messageAllowMock.mockReset().mockResolvedValue([]);
|
||||||
|
const { mentionHandler, handleSlackMessage } = createMessageHandlers({ dmPolicy: "open" });
|
||||||
|
expect(mentionHandler).toBeTruthy();
|
||||||
|
|
||||||
|
await mentionHandler!({
|
||||||
|
event: {
|
||||||
|
type: "app_mention",
|
||||||
|
channel: "D1",
|
||||||
|
channel_type: undefined,
|
||||||
|
user: "U1",
|
||||||
|
text: "<@B1> hi",
|
||||||
|
ts: "123.300",
|
||||||
|
},
|
||||||
|
body: {},
|
||||||
|
});
|
||||||
|
|
||||||
|
await mentionHandler!({
|
||||||
|
event: {
|
||||||
|
type: "app_mention",
|
||||||
|
channel: "D1",
|
||||||
|
channel_type: "channel",
|
||||||
|
user: "U1",
|
||||||
|
text: "<@B1> hi again",
|
||||||
|
ts: "123.301",
|
||||||
|
},
|
||||||
|
body: {},
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(handleSlackMessage).not.toHaveBeenCalled();
|
||||||
|
expect(messageQueueMock).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("routes channel app_mention events to the message handler", async () => {
|
||||||
|
messageQueueMock.mockClear();
|
||||||
|
messageAllowMock.mockReset().mockResolvedValue([]);
|
||||||
|
const { mentionHandler, handleSlackMessage } = createMessageHandlers({
|
||||||
|
dmPolicy: "open",
|
||||||
|
channelType: "channel",
|
||||||
|
});
|
||||||
|
expect(mentionHandler).toBeTruthy();
|
||||||
|
|
||||||
|
await mentionHandler!({
|
||||||
|
event: {
|
||||||
|
type: "app_mention",
|
||||||
|
channel: "C1",
|
||||||
|
channel_type: "channel",
|
||||||
|
user: "U1",
|
||||||
|
text: "<@B1> hello channel",
|
||||||
|
ts: "123.400",
|
||||||
|
},
|
||||||
|
body: {},
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(handleSlackMessage).toHaveBeenCalledTimes(1);
|
||||||
|
expect(handleSlackMessage).toHaveBeenCalledWith(
|
||||||
|
expect.objectContaining({
|
||||||
|
type: "app_mention",
|
||||||
|
channel: "C1",
|
||||||
|
}),
|
||||||
|
expect.objectContaining({
|
||||||
|
source: "app_mention",
|
||||||
|
wasMentioned: true,
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
expect(messageQueueMock).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ import type { SlackEventMiddlewareArgs } from "@slack/bolt";
|
|||||||
import { danger } from "../../../globals.js";
|
import { danger } from "../../../globals.js";
|
||||||
import { enqueueSystemEvent } from "../../../infra/system-events.js";
|
import { enqueueSystemEvent } from "../../../infra/system-events.js";
|
||||||
import type { SlackAppMentionEvent, SlackMessageEvent } from "../../types.js";
|
import type { SlackAppMentionEvent, SlackMessageEvent } from "../../types.js";
|
||||||
|
import { normalizeSlackChannelType } from "../context.js";
|
||||||
import type { SlackMonitorContext } from "../context.js";
|
import type { SlackMonitorContext } from "../context.js";
|
||||||
import type { SlackMessageHandler } from "../message-handler.js";
|
import type { SlackMessageHandler } from "../message-handler.js";
|
||||||
import { resolveSlackMessageSubtypeHandler } from "./message-subtype-handlers.js";
|
import { resolveSlackMessageSubtypeHandler } from "./message-subtype-handlers.js";
|
||||||
@@ -66,7 +67,7 @@ export function registerSlackMessageEvents(params: {
|
|||||||
|
|
||||||
// Skip app_mention for DMs - they're already handled by message.im event
|
// Skip app_mention for DMs - they're already handled by message.im event
|
||||||
// This prevents duplicate processing when both message and app_mention fire for DMs
|
// This prevents duplicate processing when both message and app_mention fire for DMs
|
||||||
const channelType = mention.channel_type;
|
const channelType = normalizeSlackChannelType(mention.channel_type, mention.channel);
|
||||||
if (channelType === "im" || channelType === "mpim") {
|
if (channelType === "im" || channelType === "mpim") {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user