mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-17 05:41:36 +00:00
fix(slack): restore folded IDs at Web API boundaries (#103214)
* fix(slack): preserve delivery target case Co-authored-by: 唐梓夷0668001293 <tang.ziyi@xydigit.com> * fix(slack): restore API IDs at Web API boundaries Co-authored-by: 唐梓夷0668001293 <tang.ziyi@xydigit.com> --------- Co-authored-by: 唐梓夷0668001293 <tang.ziyi@xydigit.com>
This commit is contained in:
Peter Steinberger
committed by
GitHub
GitHub
parent
fa3eb673cd
commit
ffa3d9a336
@@ -238,9 +238,14 @@ describe("handleSlackAction", () => {
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ name: "raw channel id", channelId: "C1" },
|
||||
{ name: "channel: prefixed id", channelId: "channel:C1" },
|
||||
])("adds reactions for $name", async ({ channelId }) => {
|
||||
{ name: "raw channel id", channelId: "C1", expectedChannelId: "C1" },
|
||||
{ name: "channel: prefixed id", channelId: "channel:C1", expectedChannelId: "C1" },
|
||||
{
|
||||
name: "folded channel id",
|
||||
channelId: "channel:c08gqh53ejm",
|
||||
expectedChannelId: "C08GQH53EJM",
|
||||
},
|
||||
])("adds reactions for $name", async ({ channelId, expectedChannelId }) => {
|
||||
const cfg = slackConfig();
|
||||
const result = await handleSlackAction(
|
||||
{
|
||||
@@ -251,7 +256,7 @@ describe("handleSlackAction", () => {
|
||||
},
|
||||
cfg,
|
||||
);
|
||||
expect(reactSlackMessage).toHaveBeenCalledWith("C1", "123.456", "✅", { cfg });
|
||||
expect(reactSlackMessage).toHaveBeenCalledWith(expectedChannelId, "123.456", "✅", { cfg });
|
||||
expect(JSON.parse((result.content[0] as { type: "text"; text: string }).text)).toEqual({
|
||||
ok: true,
|
||||
added: "✅",
|
||||
|
||||
@@ -34,7 +34,7 @@ import {
|
||||
getSlackExecApprovalApprovers,
|
||||
isSlackExecApprovalClientEnabled,
|
||||
} from "./exec-approvals.js";
|
||||
import { parseSlackTarget } from "./targets.js";
|
||||
import { canonicalizeSlackApiTargetId, parseSlackTarget } from "./target-parsing.js";
|
||||
|
||||
export type SlackApprovalKind = "exec" | "plugin";
|
||||
export type SlackNativeApprovalRequest = ExecApprovalRequest | PluginApprovalRequest;
|
||||
@@ -152,14 +152,14 @@ export function resolveSlackFallbackOriginTarget(
|
||||
if (!sessionTarget) {
|
||||
return null;
|
||||
}
|
||||
const parsed = parseSlackTarget(sessionTarget.id.toUpperCase(), {
|
||||
const parsed = parseSlackTarget(sessionTarget.id, {
|
||||
defaultKind: "channel",
|
||||
});
|
||||
if (!parsed) {
|
||||
return null;
|
||||
}
|
||||
return {
|
||||
to: `${parsed.kind}:${parsed.id}`,
|
||||
to: `${parsed.kind}:${canonicalizeSlackApiTargetId(parsed.kind, parsed.id)}`,
|
||||
threadId: sessionTarget.threadId,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -63,6 +63,27 @@ async function resolveExecOriginTarget(
|
||||
});
|
||||
}
|
||||
|
||||
async function resolvePluginOriginTarget(sessionKey: string) {
|
||||
return await slackApprovalCapability.native?.resolveOriginTarget?.({
|
||||
cfg: {
|
||||
...buildConfig({ allowFrom: ["U123OWNER"] }),
|
||||
session: { store: STORE_PATH },
|
||||
},
|
||||
accountId: "default",
|
||||
approvalKind: "plugin",
|
||||
request: {
|
||||
id: "plugin:req-session",
|
||||
request: {
|
||||
title: "Plugin approval",
|
||||
description: "Allow access",
|
||||
sessionKey,
|
||||
},
|
||||
createdAtMs: 0,
|
||||
expiresAtMs: 1000,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
describe("slack native approval adapter", () => {
|
||||
it("subscribes the native runtime to exec and plugin approval events", () => {
|
||||
expect(slackApprovalCapability.nativeRuntime?.eventKinds).toEqual(["exec", "plugin"]);
|
||||
@@ -773,31 +794,27 @@ describe("slack native approval adapter", () => {
|
||||
});
|
||||
|
||||
it("falls back to the session-key origin target for plugin approvals when the store is missing", async () => {
|
||||
const target = await slackApprovalCapability.native?.resolveOriginTarget?.({
|
||||
cfg: {
|
||||
...buildConfig({ allowFrom: ["U123OWNER"] }),
|
||||
session: { store: STORE_PATH },
|
||||
},
|
||||
accountId: "default",
|
||||
approvalKind: "plugin",
|
||||
request: {
|
||||
id: "plugin:req-1",
|
||||
request: {
|
||||
title: "Plugin approval",
|
||||
description: "Allow access",
|
||||
sessionKey: "agent:main:slack:channel:c123:thread:1712345678.123456",
|
||||
},
|
||||
createdAtMs: 0,
|
||||
expiresAtMs: 1000,
|
||||
},
|
||||
});
|
||||
const target = await resolvePluginOriginTarget(
|
||||
"agent:main:slack:channel:c08gqh53ejm:thread:1712345678.123456",
|
||||
);
|
||||
|
||||
expect(target).toEqual({
|
||||
to: "channel:C123",
|
||||
to: "channel:C08GQH53EJM",
|
||||
threadId: "1712345678.123456",
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves an enterprise-qualified session fallback instead of rewriting its segments", async () => {
|
||||
const target = await resolvePluginOriginTarget(
|
||||
"agent:main:slack:channel:team:T123:channel:C08GQH53EJM",
|
||||
);
|
||||
|
||||
expect(target).toEqual({
|
||||
to: "channel:team:T123:channel:C08GQH53EJM",
|
||||
threadId: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
it("skips native delivery when agent filters do not match", async () => {
|
||||
const cfg = buildConfig({
|
||||
execApprovals: {
|
||||
|
||||
@@ -548,6 +548,27 @@ describe("slackPlugin status", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("routes a folded bare W user id as a direct session", async () => {
|
||||
const resolveRoute = slackPlugin.messaging?.resolveOutboundSessionRoute;
|
||||
if (!resolveRoute) {
|
||||
throw new Error("slack messaging.resolveOutboundSessionRoute unavailable");
|
||||
}
|
||||
|
||||
const route = await resolveRoute({
|
||||
cfg: { session: { dmScope: "per-channel-peer" } } as OpenClawConfig,
|
||||
agentId: "main",
|
||||
target: "w09g2dj0275",
|
||||
});
|
||||
|
||||
expectRecordFields(route, "Slack W-user route", {
|
||||
sessionKey: "agent:main:slack:direct:w09g2dj0275",
|
||||
chatType: "direct",
|
||||
from: "slack:w09g2dj0275",
|
||||
to: "user:w09g2dj0275",
|
||||
recipientSessionExact: true,
|
||||
});
|
||||
});
|
||||
|
||||
it("canonicalizes bare Slack IM channel targets to direct user session routes", async () => {
|
||||
const resolveRoute = slackPlugin.messaging?.resolveOutboundSessionRoute;
|
||||
if (!resolveRoute) {
|
||||
@@ -572,7 +593,7 @@ describe("slackPlugin status", () => {
|
||||
},
|
||||
} as OpenClawConfig,
|
||||
agentId: "main",
|
||||
target: "D0AEWSDHAQH",
|
||||
target: "d0aewsdhaqh",
|
||||
threadId: "1778110574.653649",
|
||||
});
|
||||
|
||||
@@ -658,28 +679,57 @@ describe("slackPlugin status", () => {
|
||||
if (!resolveRoute) {
|
||||
throw new Error("slack messaging.resolveOutboundSessionRoute unavailable");
|
||||
}
|
||||
conversationsInfoMock.mockResolvedValueOnce({ channel: { id: "G123", is_mpim: true } });
|
||||
conversationsInfoMock.mockResolvedValueOnce({
|
||||
channel: { id: "G08GQH53EJM", is_mpim: true },
|
||||
});
|
||||
|
||||
const route = await resolveRoute({
|
||||
cfg: { channels: { slack: { botToken: "xoxb-test" } } } as OpenClawConfig,
|
||||
agentId: "main",
|
||||
target: "G123",
|
||||
target: "g08gqh53ejm",
|
||||
});
|
||||
|
||||
expect(conversationsInfoMock).toHaveBeenCalledWith({ channel: "G08GQH53EJM" });
|
||||
|
||||
expectRecordFields(route, "Slack MPIM route", {
|
||||
sessionKey: "agent:main:slack:group:g123",
|
||||
sessionKey: "agent:main:slack:group:g08gqh53ejm",
|
||||
chatType: "channel",
|
||||
from: "slack:group:G123",
|
||||
to: "channel:G123",
|
||||
from: "slack:group:g08gqh53ejm",
|
||||
to: "channel:g08gqh53ejm",
|
||||
recipientSessionExact: true,
|
||||
});
|
||||
expectRecordFields(requireRecord(route?.peer, "Slack MPIM peer"), "Slack MPIM peer", {
|
||||
kind: "group",
|
||||
id: "G123",
|
||||
id: "g08gqh53ejm",
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("slackPlugin messaging targets", () => {
|
||||
it("folds comparison, delivery, and session identities", () => {
|
||||
const messaging = slackPlugin.messaging;
|
||||
expect(messaging?.normalizeTarget?.("channel:C08GQH53EJM")).toBe("channel:c08gqh53ejm");
|
||||
expect(messaging?.resolveDeliveryTarget?.({ conversationId: "C08GQH53EJM" })).toEqual({
|
||||
to: "channel:c08gqh53ejm",
|
||||
});
|
||||
expect(messaging?.resolveDeliveryTarget?.({ conversationId: "c08gqh53ejm" })).toEqual({
|
||||
to: "channel:c08gqh53ejm",
|
||||
});
|
||||
expect(
|
||||
messaging?.resolveDeliveryTarget?.({
|
||||
conversationId: "1712345678.123456",
|
||||
parentConversationId: "C08GQH53EJM",
|
||||
}),
|
||||
).toEqual({
|
||||
to: "channel:c08gqh53ejm",
|
||||
threadId: "1712345678.123456",
|
||||
});
|
||||
expect(messaging?.resolveSessionTarget?.({ kind: "channel", id: "C08GQH53EJM" })).toBe(
|
||||
"channel:c08gqh53ejm",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("slackPlugin security", () => {
|
||||
it("normalizes dm allowlist entries with trimmed prefixes", () => {
|
||||
const resolveDmPolicy = slackPlugin.security?.resolveDmPolicy;
|
||||
@@ -899,7 +949,7 @@ describe("slackPlugin outbound", () => {
|
||||
it("sets and clears Slack assistant status for channel thread targets", async () => {
|
||||
const target = {
|
||||
cfg,
|
||||
to: "channel:C123",
|
||||
to: "channel:c08gqh53ejm",
|
||||
accountId: "default",
|
||||
threadId: "1712345678.123456",
|
||||
};
|
||||
@@ -910,13 +960,13 @@ describe("slackPlugin outbound", () => {
|
||||
expect(resolveSlackDmChannelIdMock).not.toHaveBeenCalled();
|
||||
expect(assistantThreadsSetStatusMock).toHaveBeenNthCalledWith(1, {
|
||||
token: "xoxb-test",
|
||||
channel_id: "C123",
|
||||
channel_id: "C08GQH53EJM",
|
||||
thread_ts: "1712345678.123456",
|
||||
status: "is typing...",
|
||||
});
|
||||
expect(assistantThreadsSetStatusMock).toHaveBeenNthCalledWith(2, {
|
||||
token: "xoxb-test",
|
||||
channel_id: "C123",
|
||||
channel_id: "C08GQH53EJM",
|
||||
thread_ts: "1712345678.123456",
|
||||
status: "",
|
||||
});
|
||||
@@ -925,14 +975,14 @@ describe("slackPlugin outbound", () => {
|
||||
it("resolves user targets to concrete DM channels for assistant status", async () => {
|
||||
await requireSlackHeartbeatSendTyping()({
|
||||
cfg,
|
||||
to: "user:U123",
|
||||
to: "user:u09g2dj0275",
|
||||
accountId: "default",
|
||||
threadId: "1712345678.123456",
|
||||
});
|
||||
|
||||
expect(resolveSlackDmChannelIdMock).toHaveBeenCalledWith({
|
||||
client: expect.any(Object),
|
||||
userId: "U123",
|
||||
userId: "U09G2DJ0275",
|
||||
accountId: "default",
|
||||
token: "xoxb-test",
|
||||
});
|
||||
|
||||
@@ -78,7 +78,7 @@ import {
|
||||
SLACK_CHANNEL,
|
||||
slackConfigAdapter,
|
||||
} from "./shared.js";
|
||||
import { parseSlackTarget } from "./target-parsing.js";
|
||||
import { canonicalizeSlackApiTargetId, parseSlackTarget } from "./target-parsing.js";
|
||||
import { slackContextTargetsMatch } from "./targets.js";
|
||||
import { normalizeSlackThreadTsCandidate, resolveSlackThreadTsValue } from "./thread-ts.js";
|
||||
import { buildSlackThreadingToolContext } from "./threading-tool-context.js";
|
||||
@@ -225,14 +225,15 @@ async function setSlackHeartbeatThreadStatus(params: {
|
||||
}
|
||||
try {
|
||||
const client = createSlackWebClient(botToken);
|
||||
const apiTargetId = canonicalizeSlackApiTargetId(target.kind, target.id, params.to);
|
||||
const channelId =
|
||||
target.kind === "channel"
|
||||
? target.id
|
||||
? apiTargetId
|
||||
: await (
|
||||
await loadSlackSendRuntime()
|
||||
).resolveSlackDmChannelId({
|
||||
client,
|
||||
userId: target.id,
|
||||
userId: apiTargetId,
|
||||
accountId: account.accountId,
|
||||
token: botToken,
|
||||
});
|
||||
@@ -357,6 +358,7 @@ async function resolveSlackOutboundSessionRoute(params: {
|
||||
if (!parsed) {
|
||||
return null;
|
||||
}
|
||||
const apiTargetId = canonicalizeSlackApiTargetId(parsed.kind, parsed.id, params.target);
|
||||
const isDm = parsed.kind === "user";
|
||||
let peerKind: "direct" | "channel" | "group" = isDm ? "direct" : "channel";
|
||||
let peerId = parsed.id;
|
||||
@@ -367,7 +369,7 @@ async function resolveSlackOutboundSessionRoute(params: {
|
||||
const conversation = await resolveSlackConversationInfo({
|
||||
cfg: params.cfg,
|
||||
accountId: params.accountId,
|
||||
channelId: parsed.id,
|
||||
channelId: apiTargetId,
|
||||
});
|
||||
if (conversation.type !== "dm" || !conversation.user) {
|
||||
return null;
|
||||
@@ -379,7 +381,7 @@ async function resolveSlackOutboundSessionRoute(params: {
|
||||
const channelType = await resolveSlackChannelType({
|
||||
cfg: params.cfg,
|
||||
accountId: params.accountId,
|
||||
channelId: parsed.id,
|
||||
channelId: apiTargetId,
|
||||
});
|
||||
if (channelType === "group") {
|
||||
peerKind = "group";
|
||||
@@ -656,14 +658,18 @@ export const slackPlugin: ChannelPlugin<ResolvedSlackAccount, SlackProbe> = crea
|
||||
messaging: {
|
||||
targetPrefixes: ["slack"],
|
||||
normalizeTarget: normalizeSlackMessagingTarget,
|
||||
// Session and delivery identities stay folded; Slack API boundaries restore ID casing.
|
||||
resolveDeliveryTarget: ({ conversationId, parentConversationId }) => {
|
||||
const parent = parentConversationId?.trim();
|
||||
const child = conversationId.trim();
|
||||
return parent && parent !== child
|
||||
? { to: `channel:${parent}`, threadId: child }
|
||||
? { to: normalizeSlackMessagingTarget(`channel:${parent}`), threadId: child }
|
||||
: { to: normalizeSlackMessagingTarget(`channel:${child}`) };
|
||||
},
|
||||
resolveSessionTarget: ({ id }) => normalizeSlackMessagingTarget(`channel:${id}`),
|
||||
resolveSessionTarget: ({ id }) => {
|
||||
// Session identities stay folded; send.ts restores unambiguous IDs at the API boundary.
|
||||
return normalizeSlackMessagingTarget(`channel:${id}`);
|
||||
},
|
||||
inferTargetChatType: ({ to }) => resolveSlackRouteTarget(to)?.chatType,
|
||||
resolveOutboundSessionRoute: async (params) => await resolveSlackOutboundSessionRoute(params),
|
||||
transformReplyPayload: ({ payload, cfg, accountId }) =>
|
||||
|
||||
@@ -146,7 +146,7 @@ describe("sendMessageSlack Enterprise listener scope", () => {
|
||||
it("uses the exact listener client without a token or team_id method payload", async () => {
|
||||
const client = createEnterpriseClient();
|
||||
|
||||
const result = await sendMessageSlack("C123", "hello", {
|
||||
const result = await sendMessageSlack("channel:c08gqh53ejm", "hello", {
|
||||
...enterpriseOptions(client),
|
||||
cfg: {
|
||||
channels: {
|
||||
@@ -162,7 +162,7 @@ describe("sendMessageSlack Enterprise listener scope", () => {
|
||||
|
||||
expect(client.chat.postMessage).toHaveBeenCalledOnce();
|
||||
expect(postPayload(client)).toEqual({
|
||||
channel: "C123",
|
||||
channel: "C08GQH53EJM",
|
||||
text: "hello",
|
||||
unfurl_links: false,
|
||||
unfurl_media: true,
|
||||
@@ -172,7 +172,7 @@ describe("sendMessageSlack Enterprise listener scope", () => {
|
||||
expect(result).toMatchObject({ messageId: "123.456", channelId: "C123" });
|
||||
});
|
||||
|
||||
it.each(["U123", "user:U123", "#general", "slack:C123"])(
|
||||
it.each(["U123", "user:U123", "#general", "slack:C123", "team:T123:channel:C08GQH53EJM"])(
|
||||
"rejects unsupported listener-owned target %s",
|
||||
async (target) => {
|
||||
const client = createEnterpriseClient();
|
||||
|
||||
@@ -11,6 +11,7 @@ vi.mock("openclaw/plugin-sdk/runtime-env", () => ({
|
||||
|
||||
const { clearSlackDefaultSendIdentitiesForTest, sendMessageSlack, setSlackDefaultSendIdentity } =
|
||||
await import("./send.js");
|
||||
const { slackPlugin } = await import("./channel.js");
|
||||
const SLACK_TEST_CFG = { channels: { slack: { botToken: "xoxb-test" } } };
|
||||
|
||||
type SlackMissingScopeError = Error & {
|
||||
@@ -92,6 +93,82 @@ describe("sendMessageSlack customize-scope fallback", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ target: "channel:c08gqh53ejm", expected: "C08GQH53EJM" },
|
||||
{ target: "c08gqh53ejm", expected: "C08GQH53EJM" },
|
||||
{ target: "user:u09g2dj0275", expected: "U09G2DJ0275" },
|
||||
{ target: "u09g2dj0275", expected: "U09G2DJ0275" },
|
||||
{ target: "@u09g2dj0275", expected: "U09G2DJ0275" },
|
||||
{ target: "user:w09g2dj0275", expected: "W09G2DJ0275" },
|
||||
{ target: "w09g2dj0275", expected: "W09G2DJ0275" },
|
||||
{ target: "companychat", expected: "companychat" },
|
||||
{ target: "channel:companychat", expected: "companychat" },
|
||||
{ target: "#companychat", expected: "companychat" },
|
||||
{ target: "#c08gqh53ejm", expected: "c08gqh53ejm" },
|
||||
{
|
||||
target: "team:T123:channel:C08GQH53EJM",
|
||||
expected: "team:T123:channel:C08GQH53EJM",
|
||||
},
|
||||
])("resolves API target $target as $expected", async ({ target, expected }) => {
|
||||
const client = createSlackSendTestClient();
|
||||
vi.mocked(client.chat.postMessage).mockResolvedValueOnce({ ts: "171234.567" });
|
||||
|
||||
await sendMessageSlack(target, "hello", {
|
||||
token: "xoxb-test",
|
||||
cfg: SLACK_TEST_CFG,
|
||||
client,
|
||||
});
|
||||
|
||||
expect(readPostMessagePayload(client, 0)).toMatchObject({ channel: expected });
|
||||
});
|
||||
|
||||
it("opens a DM with the canonical form of a folded bare user id", async () => {
|
||||
const client = createSlackSendTestClient();
|
||||
|
||||
await sendMessageSlack("u09g2dj0276", "hello", {
|
||||
token: "xoxb-test",
|
||||
cfg: SLACK_TEST_CFG,
|
||||
client,
|
||||
threadTs: "1712345678.123456",
|
||||
});
|
||||
|
||||
expect(client.conversations.open).toHaveBeenCalledWith({ users: "U09G2DJ0276" });
|
||||
});
|
||||
|
||||
it("restores a folded session target at the final send boundary", async () => {
|
||||
const client = createSlackSendTestClient();
|
||||
const target = slackPlugin.messaging?.resolveSessionTarget?.({
|
||||
kind: "channel",
|
||||
id: "c08gqh53ejm",
|
||||
});
|
||||
expect(target).toBe("channel:c08gqh53ejm");
|
||||
|
||||
await sendMessageSlack(target ?? "", "hello", {
|
||||
token: "xoxb-test",
|
||||
cfg: SLACK_TEST_CFG,
|
||||
client,
|
||||
});
|
||||
|
||||
expect(readPostMessagePayload(client, 0)).toMatchObject({ channel: "C08GQH53EJM" });
|
||||
});
|
||||
|
||||
it.each(["updates", "workspace"])(
|
||||
"keeps the channel name %s out of user-ID resolution",
|
||||
async (target) => {
|
||||
const client = createSlackSendTestClient();
|
||||
|
||||
await sendMessageSlack(target, "hello", {
|
||||
token: "xoxb-test",
|
||||
cfg: SLACK_TEST_CFG,
|
||||
client,
|
||||
threadTs: "1712345678.123456",
|
||||
});
|
||||
|
||||
expect(client.conversations.open).not.toHaveBeenCalled();
|
||||
expect(readPostMessagePayload(client, 0)).toMatchObject({ channel: target });
|
||||
},
|
||||
);
|
||||
|
||||
it("prefers an explicit send identity over the relay default", async () => {
|
||||
const client = createSlackSendTestClient();
|
||||
vi.mocked(client.chat.postMessage).mockResolvedValueOnce({ ts: "171234.567" });
|
||||
|
||||
@@ -41,7 +41,7 @@ import { assertSlackDirectSendAllowed } from "./direct-send-admission.js";
|
||||
import { markdownToSlackMrkdwnChunks } from "./format.js";
|
||||
import { SLACK_TEXT_LIMIT } from "./limits.js";
|
||||
import { recordSlackThreadParticipation } from "./sent-thread-cache.js";
|
||||
import { parseSlackTarget } from "./targets.js";
|
||||
import { canonicalizeSlackApiTargetId, parseSlackTarget } from "./target-parsing.js";
|
||||
import { normalizeSlackThreadTsCandidate, resolveSlackThreadTsValue } from "./thread-ts.js";
|
||||
import { resolveSlackBotToken } from "./token.js";
|
||||
import { truncateSlackText } from "./truncate.js";
|
||||
@@ -323,7 +323,10 @@ function parseRecipient(raw: string): SlackRecipient {
|
||||
if (!target) {
|
||||
throw new Error("Recipient is required for Slack sends");
|
||||
}
|
||||
return { kind: target.kind, id: target.id };
|
||||
return {
|
||||
kind: target.kind,
|
||||
id: canonicalizeSlackApiTargetId(target.kind, target.id, raw),
|
||||
};
|
||||
}
|
||||
|
||||
function parseEnterpriseEventRecipient(raw: string): SlackRecipient {
|
||||
@@ -331,7 +334,7 @@ function parseEnterpriseEventRecipient(raw: string): SlackRecipient {
|
||||
if (!match?.[1]) {
|
||||
throw new Error("unsupported_enterprise_slack_delivery_target");
|
||||
}
|
||||
return { kind: "channel", id: match[1] };
|
||||
return { kind: "channel", id: canonicalizeSlackApiTargetId("channel", match[1]) };
|
||||
}
|
||||
|
||||
function resolveEnterpriseEventScope(params: {
|
||||
@@ -393,8 +396,7 @@ function createSlackSendQueueKey(params: {
|
||||
threadTs?: string;
|
||||
teamId?: string;
|
||||
}): string {
|
||||
const isUserId = params.recipient.kind === "user" || /^U[A-Z0-9]+$/i.test(params.recipient.id);
|
||||
const recipientKey = `${isUserId ? "user" : params.recipient.kind}:${params.recipient.id}`;
|
||||
const recipientKey = `${params.recipient.kind}:${params.recipient.id}`;
|
||||
const workspaceScope = params.teamId ? `:${params.teamId}` : "";
|
||||
return `${params.accountId}:${createSlackTokenCacheKey(params.token)}${workspaceScope}:${recipientKey}:${
|
||||
params.threadTs ?? ""
|
||||
@@ -428,7 +430,7 @@ function setSlackDmChannelCache(key: string, channelId: string): void {
|
||||
}
|
||||
|
||||
function isSlackUserRecipient(recipient: SlackRecipient): boolean {
|
||||
return recipient.kind === "user" || /^U[A-Z0-9]+$/i.test(recipient.id);
|
||||
return recipient.kind === "user";
|
||||
}
|
||||
|
||||
function resolveDirectUserPostChannelId(params: {
|
||||
@@ -454,11 +456,10 @@ async function resolveChannelId(
|
||||
recipient: SlackRecipient,
|
||||
params: { accountId?: string; token: string },
|
||||
): Promise<{ channelId: string; isDm?: boolean; cacheHit?: boolean }> {
|
||||
// Bare Slack user IDs (U-prefix) may arrive with kind="channel" when the
|
||||
// target string had no explicit prefix (parseSlackTarget defaults bare IDs
|
||||
// to "channel"). chat.postMessage tolerates user IDs directly, but
|
||||
// Bare Slack user IDs are classified as user recipients by target parsing.
|
||||
// chat.postMessage tolerates user IDs directly, but
|
||||
// files.uploadV2 → completeUploadExternal validates channel_id against
|
||||
// ^[CGDZ][A-Z0-9]{8,}$ and rejects U-prefixed IDs. Resolve user IDs via
|
||||
// ^[CGDZ][A-Z0-9]{8,}$ and rejects user IDs. Resolve them via
|
||||
// conversations.open only for paths that require the concrete DM channel ID.
|
||||
if (!isSlackUserRecipient(recipient)) {
|
||||
return { channelId: recipient.id };
|
||||
|
||||
@@ -15,6 +15,30 @@ export type SlackTarget = MessagingTarget;
|
||||
|
||||
export type SlackTargetParseOptions = MessagingTargetParseOptions;
|
||||
|
||||
// Letter-leading folded IDs are indistinguishable from supported channel names.
|
||||
// Doctor reports that ambiguity; runtime repairs only the digit-leading form.
|
||||
const SLACK_CHANNEL_API_ID_RE = /^[CDG][0-9][A-Z0-9]{7,}$/i;
|
||||
const SLACK_USER_API_ID_RE = /^[UW][A-Z0-9]{8,}$/i;
|
||||
|
||||
function isUnambiguousSlackUserId(rawId: string): boolean {
|
||||
const id = rawId.trim();
|
||||
return /^[UW][A-Z0-9]+$/.test(id) || /^[uw][0-9][a-z0-9]{7,}$/.test(id);
|
||||
}
|
||||
|
||||
/** Restores API casing for unambiguous normalized Slack conversation IDs. */
|
||||
export function canonicalizeSlackApiTargetId(
|
||||
kind: SlackTargetKind,
|
||||
rawId: string,
|
||||
rawTarget?: string,
|
||||
): string {
|
||||
const id = rawId.trim();
|
||||
if (kind === "channel" && rawTarget?.trim().startsWith("#")) {
|
||||
return id;
|
||||
}
|
||||
const idPattern = kind === "user" ? SLACK_USER_API_ID_RE : SLACK_CHANNEL_API_ID_RE;
|
||||
return idPattern.test(id) ? id.toUpperCase() : id;
|
||||
}
|
||||
|
||||
export function parseSlackTarget(
|
||||
raw: string,
|
||||
options: SlackTargetParseOptions = {},
|
||||
@@ -46,6 +70,9 @@ export function parseSlackTarget(
|
||||
});
|
||||
return buildMessagingTarget("channel", id, trimmed);
|
||||
}
|
||||
if (isUnambiguousSlackUserId(trimmed)) {
|
||||
return buildMessagingTarget("user", trimmed, trimmed);
|
||||
}
|
||||
if (options.defaultKind) {
|
||||
return buildMessagingTarget(options.defaultKind, trimmed, trimmed);
|
||||
}
|
||||
@@ -54,7 +81,8 @@ export function parseSlackTarget(
|
||||
|
||||
export function resolveSlackChannelId(raw: string): string {
|
||||
const target = parseSlackTarget(raw, { defaultKind: "channel" });
|
||||
return requireTargetKind({ platform: "Slack", target, kind: "channel" });
|
||||
const channelId = requireTargetKind({ platform: "Slack", target, kind: "channel" });
|
||||
return canonicalizeSlackApiTargetId("channel", channelId, raw);
|
||||
}
|
||||
|
||||
export function normalizeSlackMessagingTarget(raw: string): string | undefined {
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
// Slack tests cover targets plugin behavior.
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { canonicalizeSlackApiTargetId } from "./target-parsing.js";
|
||||
import {
|
||||
normalizeSlackMessagingTarget,
|
||||
parseSlackTarget,
|
||||
@@ -14,6 +15,10 @@ describe("parseSlackTarget", () => {
|
||||
{ input: "<@U123>", id: "U123", normalized: "user:u123" },
|
||||
{ input: "user:U456", id: "U456", normalized: "user:u456" },
|
||||
{ input: "slack:U789", id: "U789", normalized: "user:u789" },
|
||||
{ input: "U2ZH3MFSR", id: "U2ZH3MFSR", normalized: "user:u2zh3mfsr" },
|
||||
{ input: "u09g2dj0275", id: "u09g2dj0275", normalized: "user:u09g2dj0275" },
|
||||
{ input: "W2ZH3MFSR", id: "W2ZH3MFSR", normalized: "user:w2zh3mfsr" },
|
||||
{ input: "w09g2dj0275", id: "w09g2dj0275", normalized: "user:w09g2dj0275" },
|
||||
] as const;
|
||||
for (const testCase of cases) {
|
||||
expect(parseSlackTarget(testCase.input), testCase.input).toEqual({
|
||||
@@ -29,6 +34,8 @@ describe("parseSlackTarget", () => {
|
||||
const cases = [
|
||||
{ input: "channel:C123", id: "C123", normalized: "channel:c123" },
|
||||
{ input: "#C999", id: "C999", normalized: "channel:c999" },
|
||||
{ input: "updates", id: "updates", normalized: "channel:updates" },
|
||||
{ input: "workspace", id: "workspace", normalized: "channel:workspace" },
|
||||
] as const;
|
||||
for (const testCase of cases) {
|
||||
expect(parseSlackTarget(testCase.input), testCase.input).toEqual({
|
||||
@@ -62,6 +69,36 @@ describe("resolveSlackChannelId", () => {
|
||||
it("rejects user targets", () => {
|
||||
expect(() => resolveSlackChannelId("user:U123")).toThrow(/channel id is required/i);
|
||||
});
|
||||
|
||||
it("restores canonical case for structurally known channel ids", () => {
|
||||
expect(resolveSlackChannelId("channel:c08gqh53ejm")).toBe("C08GQH53EJM");
|
||||
});
|
||||
|
||||
it.each(["companychat", "channel:companychat", "#companychat", "#c08gqh53ejm"])(
|
||||
"preserves the channel name %s",
|
||||
(target) => {
|
||||
expect(resolveSlackChannelId(target)).toBe(target.replace(/^(?:channel:|#)/, ""));
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
describe("Slack API target ids", () => {
|
||||
it.each([
|
||||
{ kind: "channel" as const, id: "c08gqh53ejm", expected: "C08GQH53EJM" },
|
||||
{ kind: "channel" as const, id: "d08gqh53ejm", expected: "D08GQH53EJM" },
|
||||
{ kind: "channel" as const, id: "g08gqh53ejm", expected: "G08GQH53EJM" },
|
||||
{ kind: "user" as const, id: "u09g2dj0275", expected: "U09G2DJ0275" },
|
||||
{ kind: "user" as const, id: "w09g2dj0275", expected: "W09G2DJ0275" },
|
||||
])("canonicalizes a proven $kind id", ({ kind, id, expected }) => {
|
||||
expect(canonicalizeSlackApiTargetId(kind, id)).toBe(expected);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ id: "companychat", expected: "companychat" },
|
||||
{ id: "team:T123:channel:C08GQH53EJM", expected: "team:T123:channel:C08GQH53EJM" },
|
||||
])("preserves an ambiguous channel target $id", ({ id, expected }) => {
|
||||
expect(canonicalizeSlackApiTargetId("channel", id)).toBe(expected);
|
||||
});
|
||||
});
|
||||
|
||||
describe("normalizeSlackMessagingTarget", () => {
|
||||
|
||||
Reference in New Issue
Block a user