Skip to content

Commit 50c77f2

Browse files
steipetebek91
steipete
and
bek91
committed
fix(slack): canonicalize dm mirror routes
Co-authored-by: Bek <[email protected]>
1 parent 4143c8b commit 50c77f2

5 files changed

Lines changed: 358 additions & 29 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ Docs: https://docs.openclaw.ai
1111
- Slack: add `unfurlLinks` and `unfurlMedia` config for bot `chat.postMessage` replies, including per-account overrides, so Slack link and media previews can be suppressed without workspace-wide settings. Fixes #48435. (#80145) Thanks @esegev1 and @HemantSudarshan.
1212
- Slack: add explicit `replyBroadcast` support for text and Block Kit thread replies so agents can opt into Slack's parent-channel `reply_broadcast` behavior. (#64365) Thanks @tony88331.
1313
- Slack: preserve mention target/source metadata in inbound prompt context so agents can distinguish direct bot mentions from implicit thread wakes that mention someone else. Fixes #79025. (#75356) Thanks @tmimmanuel.
14+
- Slack: canonicalize outbound delivery-mirror routes for native DM channel IDs to the peer user session so `message.send` calls to `D...` targets do not split the same Slack DM thread into a channel session. Fixes #80091. (#80111) Thanks @bek91.
1415
- Plugin SDK: deprecate public subpaths that existed for at least one month and have no bundled extension production imports, keep legacy barrel/test/zod subpath package exports for backwards compatibility, and track both sets in the SDK surface report.
1516
- Plugin SDK: deprecate public subpaths currently used by only one or two bundled plugin owners, keeping them importable while steering new plugin code to focused shared SDK seams or plugin-owned APIs.
1617
- Plugin SDK: remove the owner-specific `provider-auth-login` public subpath after moving Chutes, GitHub Copilot, and OpenAI Codex auth flows back to provider-owned modules.

‎extensions/slack/src/channel-type.test.ts‎

Lines changed: 141 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,26 @@
11
import { beforeEach, describe, expect, it, vi } from "vitest";
2-
import { __resetSlackChannelTypeCacheForTest, resolveSlackChannelType } from "./channel-type.js";
2+
import {
3+
__resetSlackChannelTypeCacheForTest,
4+
resolveSlackChannelType,
5+
resolveSlackConversationInfo,
6+
} from "./channel-type.js";
37

48
const conversationsInfoMock = vi.fn();
9+
const conversationsOpenMock = vi.fn();
510

611
vi.mock("./client.js", () => ({
712
createSlackWebClient: vi.fn(() => ({
813
conversations: {
914
info: conversationsInfoMock,
15+
open: conversationsOpenMock,
1016
},
1117
})),
1218
}));
1319

1420
describe("resolveSlackChannelType", () => {
1521
beforeEach(() => {
1622
conversationsInfoMock.mockReset();
23+
conversationsOpenMock.mockReset();
1724
__resetSlackChannelTypeCacheForTest();
1825
});
1926

@@ -58,4 +65,137 @@ describe("resolveSlackChannelType", () => {
5865

5966
expect(conversationsInfoMock).not.toHaveBeenCalled();
6067
});
68+
69+
it("returns Slack IM peer user metadata from conversations.open", async () => {
70+
conversationsOpenMock.mockResolvedValueOnce({
71+
channel: {
72+
id: "D0AEWSDHAQH",
73+
is_im: true,
74+
user: "U09G2DJ0275",
75+
},
76+
});
77+
78+
await expect(
79+
resolveSlackConversationInfo({
80+
cfg: {
81+
channels: {
82+
slack: {
83+
botToken: "xoxb-test",
84+
},
85+
},
86+
} as never,
87+
channelId: "D0AEWSDHAQH",
88+
}),
89+
).resolves.toEqual({
90+
type: "dm",
91+
user: "U09G2DJ0275",
92+
});
93+
expect(conversationsOpenMock).toHaveBeenCalledWith({
94+
channel: "D0AEWSDHAQH",
95+
prevent_creation: true,
96+
return_im: true,
97+
});
98+
expect(conversationsInfoMock).not.toHaveBeenCalled();
99+
});
100+
101+
it("keeps D-prefixed channels typed as dm when Slack lookup fails", async () => {
102+
conversationsOpenMock.mockRejectedValueOnce(new Error("missing_scope"));
103+
104+
await expect(
105+
resolveSlackConversationInfo({
106+
cfg: {
107+
channels: {
108+
slack: {
109+
botToken: "xoxb-test",
110+
},
111+
},
112+
} as never,
113+
channelId: "D0AEWSDHAQH",
114+
}),
115+
).resolves.toEqual({
116+
type: "dm",
117+
});
118+
});
119+
120+
it("does not cache incomplete native IM channel lookups", async () => {
121+
conversationsOpenMock
122+
.mockRejectedValueOnce(new Error("temporary_failure"))
123+
.mockResolvedValueOnce({
124+
channel: {
125+
id: "D0AEWSDHAQH",
126+
is_im: true,
127+
user: "U09G2DJ0275",
128+
},
129+
});
130+
131+
const cfg = {
132+
channels: {
133+
slack: {
134+
botToken: "xoxb-test",
135+
},
136+
},
137+
} as never;
138+
139+
await expect(
140+
resolveSlackConversationInfo({
141+
cfg,
142+
channelId: "D0AEWSDHAQH",
143+
}),
144+
).resolves.toEqual({
145+
type: "dm",
146+
});
147+
await expect(
148+
resolveSlackConversationInfo({
149+
cfg,
150+
channelId: "D0AEWSDHAQH",
151+
}),
152+
).resolves.toEqual({
153+
type: "dm",
154+
user: "U09G2DJ0275",
155+
});
156+
expect(conversationsOpenMock).toHaveBeenCalledTimes(2);
157+
});
158+
159+
it("does not let group-channel overrides reclassify native IM channel ids", async () => {
160+
await expect(
161+
resolveSlackConversationInfo({
162+
cfg: {
163+
channels: {
164+
slack: {
165+
dm: {
166+
groupChannels: ["D0AEWSDHAQH"],
167+
},
168+
},
169+
},
170+
} as never,
171+
channelId: "D0AEWSDHAQH",
172+
}),
173+
).resolves.toEqual({
174+
type: "dm",
175+
});
176+
expect(conversationsOpenMock).not.toHaveBeenCalled();
177+
expect(conversationsInfoMock).not.toHaveBeenCalled();
178+
});
179+
180+
it("preserves the channel-type wrapper contract", async () => {
181+
conversationsInfoMock.mockResolvedValueOnce({
182+
channel: {
183+
id: "G123",
184+
is_mpim: true,
185+
},
186+
});
187+
188+
await expect(
189+
resolveSlackChannelType({
190+
cfg: {
191+
channels: {
192+
slack: {
193+
botToken: "xoxb-test",
194+
},
195+
},
196+
} as never,
197+
channelId: "G123",
198+
}),
199+
).resolves.toBe("group");
200+
});
61201
});

‎extensions/slack/src/channel-type.ts‎

Lines changed: 62 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,38 +7,47 @@ import { createSlackWebClient } from "./client.js";
77
import { normalizeAllowListLower } from "./monitor/allow-list.js";
88
import type { OpenClawConfig } from "./runtime-api.js";
99

10-
const SLACK_CHANNEL_TYPE_CACHE = new Map<string, "channel" | "group" | "dm" | "unknown">();
10+
export type SlackConversationInfo = {
11+
type: "channel" | "group" | "dm" | "unknown";
12+
user?: string;
13+
};
1114

12-
export async function resolveSlackChannelType(params: {
15+
const SLACK_CONVERSATION_INFO_CACHE = new Map<string, SlackConversationInfo>();
16+
17+
export async function resolveSlackConversationInfo(params: {
1318
cfg: OpenClawConfig;
1419
accountId?: string | null;
1520
channelId: string;
16-
}): Promise<"channel" | "group" | "dm" | "unknown"> {
21+
}): Promise<SlackConversationInfo> {
1722
const channelId = params.channelId.trim();
1823
if (!channelId) {
19-
return "unknown";
24+
return { type: "unknown" };
2025
}
2126
const account = resolveSlackAccount({ cfg: params.cfg, accountId: params.accountId });
2227
const cacheKey = `${account.accountId}:${channelId}`;
23-
const cached = SLACK_CHANNEL_TYPE_CACHE.get(cacheKey);
28+
const cached = SLACK_CONVERSATION_INFO_CACHE.get(cacheKey);
2429
if (cached) {
2530
return cached;
2631
}
32+
const isNativeImChannel = /^D/i.test(channelId);
2733
const groupChannels = normalizeAllowListLower(account.dm?.groupChannels);
2834
const channelIdLower = normalizeLowercaseStringOrEmpty(channelId);
2935
if (
30-
groupChannels.includes(channelIdLower) ||
31-
groupChannels.includes(`slack:${channelIdLower}`) ||
32-
groupChannels.includes(`channel:${channelIdLower}`) ||
33-
groupChannels.includes(`group:${channelIdLower}`) ||
34-
groupChannels.includes(`mpim:${channelIdLower}`)
36+
!isNativeImChannel &&
37+
(groupChannels.includes(channelIdLower) ||
38+
groupChannels.includes(`slack:${channelIdLower}`) ||
39+
groupChannels.includes(`channel:${channelIdLower}`) ||
40+
groupChannels.includes(`group:${channelIdLower}`) ||
41+
groupChannels.includes(`mpim:${channelIdLower}`))
3542
) {
36-
SLACK_CHANNEL_TYPE_CACHE.set(cacheKey, "group");
37-
return "group";
43+
const result = { type: "group" } as const;
44+
SLACK_CONVERSATION_INFO_CACHE.set(cacheKey, result);
45+
return result;
3846
}
3947

4048
const channelKeys = Object.keys(account.channels ?? {});
4149
if (
50+
!isNativeImChannel &&
4251
channelKeys.some((key) => {
4352
const normalized = normalizeLowercaseStringOrEmpty(key);
4453
return (
@@ -48,32 +57,64 @@ export async function resolveSlackChannelType(params: {
4857
);
4958
})
5059
) {
51-
SLACK_CHANNEL_TYPE_CACHE.set(cacheKey, "channel");
52-
return "channel";
60+
const result = { type: "channel" } as const;
61+
SLACK_CONVERSATION_INFO_CACHE.set(cacheKey, result);
62+
return result;
5363
}
5464

5565
const token =
5666
normalizeOptionalString(account.botToken) ??
5767
normalizeOptionalString(account.config.userToken) ??
5868
"";
5969
if (!token) {
60-
SLACK_CHANNEL_TYPE_CACHE.set(cacheKey, "unknown");
61-
return "unknown";
70+
const result = { type: isNativeImChannel ? "dm" : "unknown" } as const;
71+
if (!isNativeImChannel) {
72+
SLACK_CONVERSATION_INFO_CACHE.set(cacheKey, result);
73+
}
74+
return result;
6275
}
6376

6477
try {
6578
const client = createSlackWebClient(token);
79+
if (isNativeImChannel) {
80+
const opened = await client.conversations.open({
81+
channel: channelId,
82+
prevent_creation: true,
83+
return_im: true,
84+
});
85+
const user =
86+
typeof opened.channel?.user === "string" && opened.channel.user.trim()
87+
? opened.channel.user.trim()
88+
: undefined;
89+
const result: SlackConversationInfo = user ? { type: "dm", user } : { type: "dm" };
90+
if (user) {
91+
SLACK_CONVERSATION_INFO_CACHE.set(cacheKey, result);
92+
}
93+
return result;
94+
}
6695
const info = await client.conversations.info({ channel: channelId });
6796
const channel = info.channel as { is_im?: boolean; is_mpim?: boolean } | undefined;
6897
const type = channel?.is_im ? "dm" : channel?.is_mpim ? "group" : "channel";
69-
SLACK_CHANNEL_TYPE_CACHE.set(cacheKey, type);
70-
return type;
98+
const result = { type } as const;
99+
SLACK_CONVERSATION_INFO_CACHE.set(cacheKey, result);
100+
return result;
71101
} catch {
72-
SLACK_CHANNEL_TYPE_CACHE.set(cacheKey, "unknown");
73-
return "unknown";
102+
const result = { type: isNativeImChannel ? "dm" : "unknown" } as const;
103+
if (!isNativeImChannel) {
104+
SLACK_CONVERSATION_INFO_CACHE.set(cacheKey, result);
105+
}
106+
return result;
74107
}
75108
}
76109

110+
export async function resolveSlackChannelType(params: {
111+
cfg: OpenClawConfig;
112+
accountId?: string | null;
113+
channelId: string;
114+
}): Promise<"channel" | "group" | "dm" | "unknown"> {
115+
return (await resolveSlackConversationInfo(params)).type;
116+
}
117+
77118
export function __resetSlackChannelTypeCacheForTest(): void {
78-
SLACK_CHANNEL_TYPE_CACHE.clear();
119+
SLACK_CONVERSATION_INFO_CACHE.clear();
79120
}

0 commit comments

Comments
 (0)