mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-10 10:55:07 +00:00
refactor: dedupe channel and gateway surfaces
This commit is contained in:
@@ -36,6 +36,18 @@ function createContext(overrides?: {
|
||||
} as Parameters<typeof createSlackMessageHandler>[0]["ctx"];
|
||||
}
|
||||
|
||||
function createHandlerWithTracker(overrides?: {
|
||||
markMessageSeen?: (channel: string | undefined, ts: string | undefined) => boolean;
|
||||
}) {
|
||||
const trackEvent = vi.fn();
|
||||
const handler = createSlackMessageHandler({
|
||||
ctx: createContext(overrides),
|
||||
account: { accountId: "default" } as Parameters<typeof createSlackMessageHandler>[0]["account"],
|
||||
trackEvent,
|
||||
});
|
||||
return { handler, trackEvent };
|
||||
}
|
||||
|
||||
describe("createSlackMessageHandler", () => {
|
||||
beforeEach(() => {
|
||||
enqueueMock.mockClear();
|
||||
@@ -68,14 +80,7 @@ describe("createSlackMessageHandler", () => {
|
||||
});
|
||||
|
||||
it("does not track duplicate messages that are already seen", async () => {
|
||||
const trackEvent = vi.fn();
|
||||
const handler = createSlackMessageHandler({
|
||||
ctx: createContext({ markMessageSeen: () => true }),
|
||||
account: { accountId: "default" } as Parameters<
|
||||
typeof createSlackMessageHandler
|
||||
>[0]["account"],
|
||||
trackEvent,
|
||||
});
|
||||
const { handler, trackEvent } = createHandlerWithTracker({ markMessageSeen: () => true });
|
||||
|
||||
await handler(
|
||||
{
|
||||
@@ -93,14 +98,7 @@ describe("createSlackMessageHandler", () => {
|
||||
});
|
||||
|
||||
it("tracks accepted non-duplicate messages", async () => {
|
||||
const trackEvent = vi.fn();
|
||||
const handler = createSlackMessageHandler({
|
||||
ctx: createContext(),
|
||||
account: { accountId: "default" } as Parameters<
|
||||
typeof createSlackMessageHandler
|
||||
>[0]["account"],
|
||||
trackEvent,
|
||||
});
|
||||
const { handler, trackEvent } = createHandlerWithTracker();
|
||||
|
||||
await handler(
|
||||
{
|
||||
|
||||
68
src/slack/monitor/message-handler/prepare.test-helpers.ts
Normal file
68
src/slack/monitor/message-handler/prepare.test-helpers.ts
Normal file
@@ -0,0 +1,68 @@
|
||||
import type { App } from "@slack/bolt";
|
||||
import type { OpenClawConfig } from "../../../config/config.js";
|
||||
import type { RuntimeEnv } from "../../../runtime.js";
|
||||
import type { ResolvedSlackAccount } from "../../accounts.js";
|
||||
import { createSlackMonitorContext } from "../context.js";
|
||||
|
||||
export function createInboundSlackTestContext(params: {
|
||||
cfg: OpenClawConfig;
|
||||
appClient?: App["client"];
|
||||
defaultRequireMention?: boolean;
|
||||
replyToMode?: "off" | "all" | "first";
|
||||
channelsConfig?: Record<string, { systemPrompt: string }>;
|
||||
}) {
|
||||
return createSlackMonitorContext({
|
||||
cfg: params.cfg,
|
||||
accountId: "default",
|
||||
botToken: "token",
|
||||
app: { client: params.appClient ?? {} } as App,
|
||||
runtime: {} as RuntimeEnv,
|
||||
botUserId: "B1",
|
||||
teamId: "T1",
|
||||
apiAppId: "A1",
|
||||
historyLimit: 0,
|
||||
sessionScope: "per-sender",
|
||||
mainKey: "main",
|
||||
dmEnabled: true,
|
||||
dmPolicy: "open",
|
||||
allowFrom: [],
|
||||
allowNameMatching: false,
|
||||
groupDmEnabled: true,
|
||||
groupDmChannels: [],
|
||||
defaultRequireMention: params.defaultRequireMention ?? true,
|
||||
channelsConfig: params.channelsConfig,
|
||||
groupPolicy: "open",
|
||||
useAccessGroups: false,
|
||||
reactionMode: "off",
|
||||
reactionAllowlist: [],
|
||||
replyToMode: params.replyToMode ?? "off",
|
||||
threadHistoryScope: "thread",
|
||||
threadInheritParent: false,
|
||||
slashCommand: {
|
||||
enabled: false,
|
||||
name: "openclaw",
|
||||
sessionPrefix: "slack:slash",
|
||||
ephemeral: true,
|
||||
},
|
||||
textLimit: 4000,
|
||||
ackReactionScope: "group-mentions",
|
||||
mediaMaxBytes: 1024,
|
||||
removeAckAfterReply: false,
|
||||
});
|
||||
}
|
||||
|
||||
export function createSlackTestAccount(
|
||||
config: ResolvedSlackAccount["config"] = {},
|
||||
): ResolvedSlackAccount {
|
||||
return {
|
||||
accountId: "default",
|
||||
enabled: true,
|
||||
botTokenSource: "config",
|
||||
appTokenSource: "config",
|
||||
userTokenSource: "none",
|
||||
config,
|
||||
replyToMode: config.replyToMode,
|
||||
replyToModeByChatType: config.replyToModeByChatType,
|
||||
dm: config.dm,
|
||||
};
|
||||
}
|
||||
@@ -1,64 +1,26 @@
|
||||
import type { App } from "@slack/bolt";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import type { OpenClawConfig } from "../../../config/config.js";
|
||||
import type { RuntimeEnv } from "../../../runtime.js";
|
||||
import type { ResolvedSlackAccount } from "../../accounts.js";
|
||||
import type { SlackMessageEvent } from "../../types.js";
|
||||
import { createSlackMonitorContext } from "../context.js";
|
||||
import { prepareSlackMessage } from "./prepare.js";
|
||||
import { createInboundSlackTestContext, createSlackTestAccount } from "./prepare.test-helpers.js";
|
||||
|
||||
function buildCtx(overrides?: { replyToMode?: "all" | "first" | "off" }) {
|
||||
return createSlackMonitorContext({
|
||||
const replyToMode = overrides?.replyToMode ?? "all";
|
||||
return createInboundSlackTestContext({
|
||||
cfg: {
|
||||
channels: {
|
||||
slack: { enabled: true, replyToMode: overrides?.replyToMode ?? "all" },
|
||||
slack: { enabled: true, replyToMode },
|
||||
},
|
||||
} as OpenClawConfig,
|
||||
accountId: "default",
|
||||
botToken: "token",
|
||||
app: { client: {} } as App,
|
||||
runtime: {} as RuntimeEnv,
|
||||
botUserId: "B1",
|
||||
teamId: "T1",
|
||||
apiAppId: "A1",
|
||||
historyLimit: 0,
|
||||
sessionScope: "per-sender",
|
||||
mainKey: "main",
|
||||
dmEnabled: true,
|
||||
dmPolicy: "open",
|
||||
allowFrom: [],
|
||||
groupDmEnabled: true,
|
||||
groupDmChannels: [],
|
||||
appClient: {} as App["client"],
|
||||
defaultRequireMention: false,
|
||||
groupPolicy: "open",
|
||||
allowNameMatching: false,
|
||||
useAccessGroups: false,
|
||||
reactionMode: "off",
|
||||
reactionAllowlist: [],
|
||||
replyToMode: overrides?.replyToMode ?? "all",
|
||||
threadHistoryScope: "thread",
|
||||
threadInheritParent: false,
|
||||
slashCommand: {
|
||||
enabled: false,
|
||||
name: "openclaw",
|
||||
sessionPrefix: "slack:slash",
|
||||
ephemeral: true,
|
||||
},
|
||||
textLimit: 4000,
|
||||
ackReactionScope: "group-mentions",
|
||||
mediaMaxBytes: 1024,
|
||||
removeAckAfterReply: false,
|
||||
replyToMode,
|
||||
});
|
||||
}
|
||||
|
||||
const account: ResolvedSlackAccount = {
|
||||
accountId: "default",
|
||||
enabled: true,
|
||||
botTokenSource: "config",
|
||||
appTokenSource: "config",
|
||||
userTokenSource: "none",
|
||||
config: {},
|
||||
};
|
||||
const account: ResolvedSlackAccount = createSlackTestAccount();
|
||||
|
||||
describe("thread-level session keys", () => {
|
||||
it("uses thread-level session key for channel messages", async () => {
|
||||
|
||||
@@ -510,11 +510,11 @@ export async function registerSlackMonitorSlashCommands(params: {
|
||||
const [
|
||||
{ resolveConversationLabel },
|
||||
{ createReplyPrefixOptions },
|
||||
{ recordSessionMetaFromInbound, resolveStorePath },
|
||||
{ recordInboundSessionMetaSafe },
|
||||
] = await Promise.all([
|
||||
import("../../channels/conversation-label.js"),
|
||||
import("../../channels/reply-prefix.js"),
|
||||
import("../../config/sessions.js"),
|
||||
import("../../channels/session-meta.js"),
|
||||
]);
|
||||
|
||||
const route = resolveAgentRoute({
|
||||
@@ -578,18 +578,14 @@ export async function registerSlackMonitorSlashCommands(params: {
|
||||
OriginatingTo: `user:${command.user_id}`,
|
||||
});
|
||||
|
||||
const storePath = resolveStorePath(cfg.session?.store, {
|
||||
await recordInboundSessionMetaSafe({
|
||||
cfg,
|
||||
agentId: route.agentId,
|
||||
sessionKey: ctxPayload.SessionKey ?? route.sessionKey,
|
||||
ctx: ctxPayload,
|
||||
onError: (err) =>
|
||||
runtime.error?.(danger(`slack slash: failed updating session meta: ${String(err)}`)),
|
||||
});
|
||||
try {
|
||||
await recordSessionMetaFromInbound({
|
||||
storePath,
|
||||
sessionKey: ctxPayload.SessionKey ?? route.sessionKey,
|
||||
ctx: ctxPayload,
|
||||
});
|
||||
} catch (err) {
|
||||
runtime.error?.(danger(`slack slash: failed updating session meta: ${String(err)}`));
|
||||
}
|
||||
|
||||
const { onModelSelected, ...prefixOptions } = createReplyPrefixOptions({
|
||||
cfg,
|
||||
|
||||
@@ -1,8 +1,7 @@
|
||||
import {
|
||||
buildMessagingTarget,
|
||||
ensureTargetId,
|
||||
parseTargetMention,
|
||||
parseTargetPrefixes,
|
||||
parseMentionPrefixOrAtUserTarget,
|
||||
requireTargetKind,
|
||||
type MessagingTarget,
|
||||
type MessagingTargetKind,
|
||||
@@ -23,33 +22,19 @@ export function parseSlackTarget(
|
||||
if (!trimmed) {
|
||||
return undefined;
|
||||
}
|
||||
const mentionTarget = parseTargetMention({
|
||||
const userTarget = parseMentionPrefixOrAtUserTarget({
|
||||
raw: trimmed,
|
||||
mentionPattern: /^<@([A-Z0-9]+)>$/i,
|
||||
kind: "user",
|
||||
});
|
||||
if (mentionTarget) {
|
||||
return mentionTarget;
|
||||
}
|
||||
const prefixedTarget = parseTargetPrefixes({
|
||||
raw: trimmed,
|
||||
prefixes: [
|
||||
{ prefix: "user:", kind: "user" },
|
||||
{ prefix: "channel:", kind: "channel" },
|
||||
{ prefix: "slack:", kind: "user" },
|
||||
],
|
||||
atUserPattern: /^[A-Z0-9]+$/i,
|
||||
atUserErrorMessage: "Slack DMs require a user id (use user:<id> or <@id>)",
|
||||
});
|
||||
if (prefixedTarget) {
|
||||
return prefixedTarget;
|
||||
}
|
||||
if (trimmed.startsWith("@")) {
|
||||
const candidate = trimmed.slice(1).trim();
|
||||
const id = ensureTargetId({
|
||||
candidate,
|
||||
pattern: /^[A-Z0-9]+$/i,
|
||||
errorMessage: "Slack DMs require a user id (use user:<id> or <@id>)",
|
||||
});
|
||||
return buildMessagingTarget("user", id, trimmed);
|
||||
if (userTarget) {
|
||||
return userTarget;
|
||||
}
|
||||
if (trimmed.startsWith("#")) {
|
||||
const candidate = trimmed.slice(1).trim();
|
||||
|
||||
@@ -4,6 +4,23 @@ import { buildSlackThreadingToolContext } from "./threading-tool-context.js";
|
||||
|
||||
const emptyCfg = {} as OpenClawConfig;
|
||||
|
||||
function resolveReplyToModeWithConfig(params: {
|
||||
slackConfig: Record<string, unknown>;
|
||||
context: Record<string, unknown>;
|
||||
}) {
|
||||
const cfg = {
|
||||
channels: {
|
||||
slack: params.slackConfig,
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
const result = buildSlackThreadingToolContext({
|
||||
cfg,
|
||||
accountId: null,
|
||||
context: params.context as never,
|
||||
});
|
||||
return result.replyToMode;
|
||||
}
|
||||
|
||||
describe("buildSlackThreadingToolContext", () => {
|
||||
it("uses top-level replyToMode by default", () => {
|
||||
const cfg = {
|
||||
@@ -20,37 +37,27 @@ describe("buildSlackThreadingToolContext", () => {
|
||||
});
|
||||
|
||||
it("uses chat-type replyToMode overrides for direct messages when configured", () => {
|
||||
const cfg = {
|
||||
channels: {
|
||||
slack: {
|
||||
expect(
|
||||
resolveReplyToModeWithConfig({
|
||||
slackConfig: {
|
||||
replyToMode: "off",
|
||||
replyToModeByChatType: { direct: "all" },
|
||||
},
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
const result = buildSlackThreadingToolContext({
|
||||
cfg,
|
||||
accountId: null,
|
||||
context: { ChatType: "direct" },
|
||||
});
|
||||
expect(result.replyToMode).toBe("all");
|
||||
context: { ChatType: "direct" },
|
||||
}),
|
||||
).toBe("all");
|
||||
});
|
||||
|
||||
it("uses top-level replyToMode for channels when no channel override is set", () => {
|
||||
const cfg = {
|
||||
channels: {
|
||||
slack: {
|
||||
expect(
|
||||
resolveReplyToModeWithConfig({
|
||||
slackConfig: {
|
||||
replyToMode: "off",
|
||||
replyToModeByChatType: { direct: "all" },
|
||||
},
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
const result = buildSlackThreadingToolContext({
|
||||
cfg,
|
||||
accountId: null,
|
||||
context: { ChatType: "channel" },
|
||||
});
|
||||
expect(result.replyToMode).toBe("off");
|
||||
context: { ChatType: "channel" },
|
||||
}),
|
||||
).toBe("off");
|
||||
});
|
||||
|
||||
it("falls back to top-level when no chat-type override is set", () => {
|
||||
@@ -70,61 +77,46 @@ describe("buildSlackThreadingToolContext", () => {
|
||||
});
|
||||
|
||||
it("uses legacy dm.replyToMode for direct messages when no chat-type override exists", () => {
|
||||
const cfg = {
|
||||
channels: {
|
||||
slack: {
|
||||
expect(
|
||||
resolveReplyToModeWithConfig({
|
||||
slackConfig: {
|
||||
replyToMode: "off",
|
||||
dm: { replyToMode: "all" },
|
||||
},
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
const result = buildSlackThreadingToolContext({
|
||||
cfg,
|
||||
accountId: null,
|
||||
context: { ChatType: "direct" },
|
||||
});
|
||||
expect(result.replyToMode).toBe("all");
|
||||
context: { ChatType: "direct" },
|
||||
}),
|
||||
).toBe("all");
|
||||
});
|
||||
|
||||
it("uses all mode when MessageThreadId is present", () => {
|
||||
const cfg = {
|
||||
channels: {
|
||||
slack: {
|
||||
expect(
|
||||
resolveReplyToModeWithConfig({
|
||||
slackConfig: {
|
||||
replyToMode: "all",
|
||||
replyToModeByChatType: { direct: "off" },
|
||||
},
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
const result = buildSlackThreadingToolContext({
|
||||
cfg,
|
||||
accountId: null,
|
||||
context: {
|
||||
ChatType: "direct",
|
||||
ThreadLabel: "thread-label",
|
||||
MessageThreadId: "1771999998.834199",
|
||||
},
|
||||
});
|
||||
expect(result.replyToMode).toBe("all");
|
||||
context: {
|
||||
ChatType: "direct",
|
||||
ThreadLabel: "thread-label",
|
||||
MessageThreadId: "1771999998.834199",
|
||||
},
|
||||
}),
|
||||
).toBe("all");
|
||||
});
|
||||
|
||||
it("does not force all mode from ThreadLabel alone", () => {
|
||||
const cfg = {
|
||||
channels: {
|
||||
slack: {
|
||||
expect(
|
||||
resolveReplyToModeWithConfig({
|
||||
slackConfig: {
|
||||
replyToMode: "all",
|
||||
replyToModeByChatType: { direct: "off" },
|
||||
},
|
||||
},
|
||||
} as OpenClawConfig;
|
||||
const result = buildSlackThreadingToolContext({
|
||||
cfg,
|
||||
accountId: null,
|
||||
context: {
|
||||
ChatType: "direct",
|
||||
ThreadLabel: "label-without-real-thread",
|
||||
},
|
||||
});
|
||||
expect(result.replyToMode).toBe("off");
|
||||
context: {
|
||||
ChatType: "direct",
|
||||
ThreadLabel: "label-without-real-thread",
|
||||
},
|
||||
}),
|
||||
).toBe("off");
|
||||
});
|
||||
|
||||
it("keeps configured channel behavior when not in a thread", () => {
|
||||
|
||||
@@ -2,6 +2,22 @@ import { describe, expect, it } from "vitest";
|
||||
import { resolveSlackThreadContext, resolveSlackThreadTargets } from "./threading.js";
|
||||
|
||||
describe("resolveSlackThreadTargets", () => {
|
||||
function expectAutoCreatedTopLevelThreadTsBehavior(replyToMode: "off" | "first") {
|
||||
const { replyThreadTs, statusThreadTs, isThreadReply } = resolveSlackThreadTargets({
|
||||
replyToMode,
|
||||
message: {
|
||||
type: "message",
|
||||
channel: "C1",
|
||||
ts: "123",
|
||||
thread_ts: "123",
|
||||
},
|
||||
});
|
||||
|
||||
expect(isThreadReply).toBe(false);
|
||||
expect(replyThreadTs).toBeUndefined();
|
||||
expect(statusThreadTs).toBeUndefined();
|
||||
}
|
||||
|
||||
it("threads replies when message is already threaded", () => {
|
||||
const { replyThreadTs, statusThreadTs } = resolveSlackThreadTargets({
|
||||
replyToMode: "off",
|
||||
@@ -46,35 +62,11 @@ describe("resolveSlackThreadTargets", () => {
|
||||
});
|
||||
|
||||
it("does not treat auto-created top-level thread_ts as a real thread when mode is off", () => {
|
||||
const { replyThreadTs, statusThreadTs, isThreadReply } = resolveSlackThreadTargets({
|
||||
replyToMode: "off",
|
||||
message: {
|
||||
type: "message",
|
||||
channel: "C1",
|
||||
ts: "123",
|
||||
thread_ts: "123",
|
||||
},
|
||||
});
|
||||
|
||||
expect(isThreadReply).toBe(false);
|
||||
expect(replyThreadTs).toBeUndefined();
|
||||
expect(statusThreadTs).toBeUndefined();
|
||||
expectAutoCreatedTopLevelThreadTsBehavior("off");
|
||||
});
|
||||
|
||||
it("keeps first-mode behavior for auto-created top-level thread_ts", () => {
|
||||
const { replyThreadTs, statusThreadTs, isThreadReply } = resolveSlackThreadTargets({
|
||||
replyToMode: "first",
|
||||
message: {
|
||||
type: "message",
|
||||
channel: "C1",
|
||||
ts: "123",
|
||||
thread_ts: "123",
|
||||
},
|
||||
});
|
||||
|
||||
expect(isThreadReply).toBe(false);
|
||||
expect(replyThreadTs).toBeUndefined();
|
||||
expect(statusThreadTs).toBeUndefined();
|
||||
expectAutoCreatedTopLevelThreadTsBehavior("first");
|
||||
});
|
||||
|
||||
it("sets messageThreadId for top-level messages when replyToMode is all", () => {
|
||||
|
||||
Reference in New Issue
Block a user