From f33d0a884e96827e8a48fabc4d6b1300287749b6 Mon Sep 17 00:00:00 2001 From: Marcus Castro Date: Tue, 24 Feb 2026 10:51:37 -0300 Subject: [PATCH] fix(slack): override wrong channel_type for D-prefix DM channels --- src/slack/monitor/context.ts | 7 +- .../monitor/message-handler/prepare.test.ts | 158 ++++++++++++++++++ src/slack/monitor/monitor.test.ts | 19 +++ 3 files changed, 183 insertions(+), 1 deletion(-) diff --git a/src/slack/monitor/context.ts b/src/slack/monitor/context.ts index ecf04974937..f43f77a0d76 100644 --- a/src/slack/monitor/context.ts +++ b/src/slack/monitor/context.ts @@ -38,15 +38,20 @@ export function normalizeSlackChannelType( channelId?: string | null, ): SlackMessageEvent["channel_type"] { const normalized = channelType?.trim().toLowerCase(); + const inferred = inferSlackChannelType(channelId); if ( normalized === "im" || normalized === "mpim" || normalized === "channel" || normalized === "group" ) { + // D-prefix channel IDs are always DMs — override a contradicting channel_type. + if (inferred === "im" && normalized !== "im") { + return "im"; + } return normalized; } - return inferSlackChannelType(channelId) ?? "channel"; + return inferred ?? "channel"; } export type SlackMonitorContext = { diff --git a/src/slack/monitor/message-handler/prepare.test.ts b/src/slack/monitor/message-handler/prepare.test.ts index 07e6b834501..654b02f3ce1 100644 --- a/src/slack/monitor/message-handler/prepare.test.ts +++ b/src/slack/monitor/message-handler/prepare.test.ts @@ -264,6 +264,164 @@ describe("slack prepareSlackMessage inbound contract", () => { expect(untrusted).toContain("Do dangerous things"); }); + it("classifies D-prefix DMs correctly even when channel_type is wrong", async () => { + const slackCtx = createSlackMonitorContext({ + cfg: { + channels: { slack: { enabled: true } }, + session: { dmScope: "main" }, + } 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: [], + defaultRequireMention: true, + groupPolicy: "open", + useAccessGroups: false, + reactionMode: "off", + reactionAllowlist: [], + 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, + }); + // oxlint-disable-next-line typescript/no-explicit-any + slackCtx.resolveUserName = async () => ({ name: "Alice" }) as any; + // Simulate API returning correct type for DM channel + slackCtx.resolveChannelName = async () => ({ name: undefined, type: "im" as const }); + + const account: ResolvedSlackAccount = { + accountId: "default", + enabled: true, + botTokenSource: "config", + appTokenSource: "config", + config: {}, + }; + + // Bug scenario: D-prefix channel but Slack event says channel_type: "channel" + const message: SlackMessageEvent = { + channel: "D0ACP6B1T8V", + channel_type: "channel", + user: "U1", + text: "hello from DM", + ts: "1.000", + } as SlackMessageEvent; + + const prepared = await prepareSlackMessage({ + ctx: slackCtx, + account, + message, + opts: { source: "message" }, + }); + + expect(prepared).toBeTruthy(); + // oxlint-disable-next-line typescript/no-explicit-any + expectInboundContextContract(prepared!.ctxPayload as any); + // Should be classified as DM, not channel + expect(prepared!.isDirectMessage).toBe(true); + // DM with dmScope: "main" should route to the main session + expect(prepared!.route.sessionKey).toBe("agent:main:main"); + // ChatType should be "direct", not "channel" + expect(prepared!.ctxPayload.ChatType).toBe("direct"); + // From should use user ID (DM pattern), not channel ID + expect(prepared!.ctxPayload.From).toContain("slack:U1"); + }); + + it("classifies D-prefix DMs when channel_type is missing", async () => { + const slackCtx = createSlackMonitorContext({ + cfg: { + channels: { slack: { enabled: true } }, + session: { dmScope: "main" }, + } 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: [], + defaultRequireMention: true, + groupPolicy: "open", + useAccessGroups: false, + reactionMode: "off", + reactionAllowlist: [], + 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, + }); + // oxlint-disable-next-line typescript/no-explicit-any + slackCtx.resolveUserName = async () => ({ name: "Alice" }) as any; + // Simulate API returning correct type for DM channel + slackCtx.resolveChannelName = async () => ({ name: undefined, type: "im" as const }); + + const account: ResolvedSlackAccount = { + accountId: "default", + enabled: true, + botTokenSource: "config", + appTokenSource: "config", + config: {}, + }; + + // channel_type missing — should infer from D-prefix + const message: SlackMessageEvent = { + channel: "D0ACP6B1T8V", + user: "U1", + text: "hello from DM", + ts: "1.000", + } as SlackMessageEvent; + + const prepared = await prepareSlackMessage({ + ctx: slackCtx, + account, + message, + opts: { source: "message" }, + }); + + expect(prepared).toBeTruthy(); + // oxlint-disable-next-line typescript/no-explicit-any + expectInboundContextContract(prepared!.ctxPayload as any); + expect(prepared!.isDirectMessage).toBe(true); + expect(prepared!.route.sessionKey).toBe("agent:main:main"); + expect(prepared!.ctxPayload.ChatType).toBe("direct"); + }); + it("sets MessageThreadId for top-level messages when replyToMode=all", async () => { const slackCtx = createInboundSlackCtx({ cfg: { diff --git a/src/slack/monitor/monitor.test.ts b/src/slack/monitor/monitor.test.ts index 2a6072d93dd..3262873718d 100644 --- a/src/slack/monitor/monitor.test.ts +++ b/src/slack/monitor/monitor.test.ts @@ -134,6 +134,25 @@ describe("normalizeSlackChannelType", () => { it("prefers explicit channel_type values", () => { expect(normalizeSlackChannelType("mpim", "C123")).toBe("mpim"); }); + + it("overrides wrong channel_type for D-prefix DM channels", () => { + // Slack DM channel IDs always start with "D" — if the event + // reports a wrong channel_type, the D-prefix should win. + expect(normalizeSlackChannelType("channel", "D123")).toBe("im"); + expect(normalizeSlackChannelType("group", "D456")).toBe("im"); + expect(normalizeSlackChannelType("mpim", "D789")).toBe("im"); + }); + + it("preserves correct channel_type for D-prefix DM channels", () => { + expect(normalizeSlackChannelType("im", "D123")).toBe("im"); + }); + + it("does not override G-prefix channel_type (ambiguous prefix)", () => { + // G-prefix can be either "group" (private channel) or "mpim" (group DM) + // — trust the provided channel_type since the prefix is ambiguous. + expect(normalizeSlackChannelType("group", "G123")).toBe("group"); + expect(normalizeSlackChannelType("mpim", "G456")).toBe("mpim"); + }); }); describe("resolveSlackSystemEventSessionKey", () => {