diff --git a/extensions/slack/src/action-runtime.test.ts b/extensions/slack/src/action-runtime.test.ts index de6c9488a57b..f11d5ee2031c 100644 --- a/extensions/slack/src/action-runtime.test.ts +++ b/extensions/slack/src/action-runtime.test.ts @@ -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: "✅", diff --git a/extensions/slack/src/approval-native-gates.ts b/extensions/slack/src/approval-native-gates.ts index 7ab0483d8ecd..cb982d10f3cc 100644 --- a/extensions/slack/src/approval-native-gates.ts +++ b/extensions/slack/src/approval-native-gates.ts @@ -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, }; } diff --git a/extensions/slack/src/approval-native.test.ts b/extensions/slack/src/approval-native.test.ts index 5f35dc9926d7..106311469557 100644 --- a/extensions/slack/src/approval-native.test.ts +++ b/extensions/slack/src/approval-native.test.ts @@ -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: { diff --git a/extensions/slack/src/channel.test.ts b/extensions/slack/src/channel.test.ts index 8e84eadb4579..127be9ca715b 100644 --- a/extensions/slack/src/channel.test.ts +++ b/extensions/slack/src/channel.test.ts @@ -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", }); diff --git a/extensions/slack/src/channel.ts b/extensions/slack/src/channel.ts index 9f70959294f8..8016b1c32dc4 100644 --- a/extensions/slack/src/channel.ts +++ b/extensions/slack/src/channel.ts @@ -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 = 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 }) => diff --git a/extensions/slack/src/send.enterprise.test.ts b/extensions/slack/src/send.enterprise.test.ts index 5191846d3d1e..3de5538f97aa 100644 --- a/extensions/slack/src/send.enterprise.test.ts +++ b/extensions/slack/src/send.enterprise.test.ts @@ -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(); diff --git a/extensions/slack/src/send.identity-fallback.test.ts b/extensions/slack/src/send.identity-fallback.test.ts index 6136397b97b7..02ad99d3bb42 100644 --- a/extensions/slack/src/send.identity-fallback.test.ts +++ b/extensions/slack/src/send.identity-fallback.test.ts @@ -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" }); diff --git a/extensions/slack/src/send.ts b/extensions/slack/src/send.ts index c4f975a113a2..e3c11cfade57 100644 --- a/extensions/slack/src/send.ts +++ b/extensions/slack/src/send.ts @@ -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 }; diff --git a/extensions/slack/src/target-parsing.ts b/extensions/slack/src/target-parsing.ts index 7d62c4a0185f..957284f61a6f 100644 --- a/extensions/slack/src/target-parsing.ts +++ b/extensions/slack/src/target-parsing.ts @@ -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 { diff --git a/extensions/slack/src/targets.test.ts b/extensions/slack/src/targets.test.ts index af3ee72b24df..0e9321b2192b 100644 --- a/extensions/slack/src/targets.test.ts +++ b/extensions/slack/src/targets.test.ts @@ -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", () => {