diff --git a/docs/channels/slack.md b/docs/channels/slack.md index dae0b8b0a255..ee0d8eafdbd7 100644 --- a/docs/channels/slack.md +++ b/docs/channels/slack.md @@ -240,14 +240,21 @@ bot-authored `message` and `app_mention` events before dispatch, regardless of `allowBots`, because org installs do not provide a stable workspace-qualified bot identity for loop prevention. -Enterprise support is intentionally limited to direct Socket Mode or HTTP -`message` and `app_mention` events and their immediate replies. Relay mode, -slash commands, interactions, App Home, reaction event listeners, pins, Slack -action tools, Slack-native approvals, bindings, queued or scheduled delivery, -and proactive sends are unavailable for an enterprise account. Outbound -acknowledgment, typing, and status reactions are supported through the -listener-owned Slack client and require `reactions:write`; inbound reaction -notifications and reaction action tools remain unavailable. +Enterprise support accepts direct Socket Mode or HTTP `message` and +`app_mention` events plus workspace-qualified outbound messages. Relay mode, +slash commands, interactions, App Home, reaction event listeners, pins, +Slack-native approvals, and bindings remain unavailable for an enterprise +account. Slack action tools remain unavailable except for file uploads and +adding or removing emoji reactions. Outbound acknowledgment, typing, and +status reactions are supported and require `reactions:write`; inbound reaction +notifications remain unavailable. + +OpenClaw records Enterprise Grid destinations as +`team::channel:` or `team::user:`. +Current-conversation sends, uploads, and reactions inherit that destination. +Detached or proactive calls must provide the workspace-qualified target; +bare channel and user IDs fail closed because those IDs can be reused by +different workspaces. Immediate replies reuse the standard Slack delivery behavior for chunks, media, metadata, identity fallback, unfurls, and receipts, but only while the diff --git a/extensions/slack/src/action-runtime.test.ts b/extensions/slack/src/action-runtime.test.ts index e35d9efa8fdf..a344c63ecdcf 100644 --- a/extensions/slack/src/action-runtime.test.ts +++ b/extensions/slack/src/action-runtime.test.ts @@ -64,14 +64,245 @@ describe("handleSlackAction", () => { } as OpenClawConfig; } - it("rejects all actions before Slack API work for an enterprise org account", async () => { + it("reads pins from the trusted Enterprise Grid workspace", async () => { + const cfg = slackConfig({ enterpriseOrgInstall: true }); + listSlackPins.mockResolvedValueOnce([]); + + const result = await handleSlackAction({ action: "listPins", channelId: "C123" }, cfg, { + currentChannelProvider: "slack", + currentChannelId: "team:T123:channel:C123", + requesterAccountId: "default", + }); + + expect(requireDetails(result).ok).toBe(true); + expect(requireMockArg(listSlackPins, "listSlackPins", 0, 0)).toBe("C123"); + expectRecordFields(requireRecordArg(listSlackPins, "listSlackPins", 0, 1), { + cfg, + teamId: "T123", + }); + }); + + it("reads the current requester from the trusted Enterprise Grid workspace", async () => { + const cfg = slackConfig({ enterpriseOrgInstall: true }); + getSlackMemberInfo.mockResolvedValueOnce({ + ok: true, + user: { id: "U123", is_bot: false }, + }); + + const result = await handleSlackAction({ action: "memberInfo", userId: "U123" }, cfg, { + currentChannelProvider: "slack", + currentChannelId: "team:T123:channel:C123", + requesterAccountId: "default", + requesterSenderId: "U123", + }); + + expect(getSlackMemberInfo).toHaveBeenCalledWith("U123", { + cfg, + teamId: "T123", + }); + expect(requireDetails(result)).toEqual({ + ok: true, + info: { ok: true, user: { id: "U123", is_bot: false } }, + }); + }); + + it.each([ + { + name: "provider does not match", + context: { + currentChannelProvider: "discord", + currentChannelId: "team:T123:channel:C123", + requesterAccountId: "default", + requesterSenderId: "U123", + }, + }, + { + name: "account does not match", + context: { + currentChannelProvider: "slack", + currentChannelId: "team:T123:channel:C123", + requesterAccountId: "other", + requesterSenderId: "U123", + }, + }, + { + name: "workspace is absent", + context: { + currentChannelProvider: "slack", + currentChannelId: "channel:C123", + requesterAccountId: "default", + requesterSenderId: "U123", + }, + }, + { + name: "current targets disagree on workspace", + context: { + currentChannelProvider: "slack", + currentChannelId: "team:T123:channel:C123", + currentMessagingTarget: "team:T456:channel:C123", + requesterAccountId: "default", + requesterSenderId: "U123", + }, + }, + ])("rejects Enterprise Grid member info when the trusted $name", async ({ context }) => { await expect( handleSlackAction( - { action: "readMessages", channelId: "C123" }, + { action: "memberInfo", userId: "U123" }, slackConfig({ enterpriseOrgInstall: true }), + context, ), - ).rejects.toThrow(/unavailable for Enterprise Grid org installs/); - expect(readSlackMessages).not.toHaveBeenCalled(); + ).rejects.toThrow(); + expect(getSlackMemberInfo).not.toHaveBeenCalled(); + }); + + it("scopes every Enterprise Grid message and pin write to the trusted current workspace", async () => { + const cfg = slackConfig({ enterpriseOrgInstall: true }); + const context = { + currentChannelProvider: "slack", + currentChannelId: "team:T123:channel:C123", + requesterAccountId: "default", + }; + + await handleSlackAction( + { action: "sendMessage", to: "channel:C123", content: "created" }, + cfg, + context, + ); + await handleSlackAction( + { + action: "editMessage", + channelId: "C123", + messageId: "123.456", + content: "updated", + }, + cfg, + context, + ); + await handleSlackAction( + { action: "deleteMessage", channelId: "C123", messageId: "123.456" }, + cfg, + context, + ); + await handleSlackAction( + { action: "pinMessage", channelId: "C123", messageId: "123.456" }, + cfg, + context, + ); + await handleSlackAction( + { action: "unpinMessage", channelId: "C123", messageId: "123.456" }, + cfg, + context, + ); + + expectSlackSendCall(0, "team:T123:channel:C123", "created", { + cfg, + mediaUrl: undefined, + threadTs: undefined, + blocks: undefined, + }); + expect(editSlackMessage).toHaveBeenCalledWith("C123", "123.456", "updated", { + cfg, + teamId: "T123", + blocks: undefined, + }); + expect(deleteSlackMessage).toHaveBeenCalledWith("C123", "123.456", { + cfg, + teamId: "T123", + }); + expect(pinSlackMessage).toHaveBeenCalledWith("C123", "123.456", { + cfg, + teamId: "T123", + }); + expect(unpinSlackMessage).toHaveBeenCalledWith("C123", "123.456", { + cfg, + teamId: "T123", + }); + }); + + it("scopes Enterprise Grid history, file, reaction, and emoji reads to the trusted workspace", async () => { + const cfg = slackConfig({ enterpriseOrgInstall: true }); + const context = { + currentChannelProvider: "slack", + currentChannelId: "team:T123:channel:C123", + requesterAccountId: "default", + }; + readSlackMessages.mockResolvedValueOnce({ messages: [], hasMore: false }); + listSlackReactions.mockResolvedValueOnce([]); + downloadSlackFile.mockResolvedValueOnce(null); + listSlackEmojis.mockResolvedValueOnce({ ok: true, emoji: { openai: "url" } }); + + await handleSlackAction({ action: "readMessages", channelId: "C123" }, cfg, context); + await handleSlackAction( + { action: "reactions", channelId: "C123", messageId: "123.456" }, + cfg, + context, + ); + await handleSlackAction( + { action: "downloadFile", channelId: "C123", fileId: "F123" }, + cfg, + context, + ); + await handleSlackAction({ action: "emojiList" }, cfg, context); + + expect(readSlackMessages).toHaveBeenCalledWith( + "C123", + expect.objectContaining({ cfg, teamId: "T123" }), + ); + expect(listSlackReactions).toHaveBeenCalledWith("C123", "123.456", { + cfg, + teamId: "T123", + }); + expect(downloadSlackFile).toHaveBeenCalledWith( + "F123", + expect.objectContaining({ cfg, teamId: "T123", channelId: "C123" }), + ); + expect(listSlackEmojis).toHaveBeenCalledWith({ cfg, teamId: "T123" }); + }); + + it.each([ + { name: "history", params: { action: "readMessages", channelId: "C123" } }, + { + name: "reactions", + params: { action: "reactions", channelId: "C123", messageId: "123.456" }, + }, + { name: "file", params: { action: "downloadFile", channelId: "C123", fileId: "F123" } }, + { name: "emoji", params: { action: "emojiList" } }, + ])("rejects a bare Enterprise Grid $name read without trusted context", async ({ params }) => { + await expect( + handleSlackAction(params, slackConfig({ enterpriseOrgInstall: true })), + ).rejects.toThrow("unsupported_enterprise_slack_delivery"); + }); + + it.each([ + { + name: "send", + params: { action: "sendMessage", to: "channel:C123", content: "created" }, + }, + { + name: "edit", + params: { + action: "editMessage", + channelId: "C123", + messageId: "123.456", + content: "updated", + }, + }, + { + name: "delete", + params: { action: "deleteMessage", channelId: "C123", messageId: "123.456" }, + }, + { + name: "pin", + params: { action: "pinMessage", channelId: "C123", messageId: "123.456" }, + }, + { + name: "unpin", + params: { action: "unpinMessage", channelId: "C123", messageId: "123.456" }, + }, + ])("rejects a bare Enterprise Grid $name without trusted current context", async ({ params }) => { + await expect( + handleSlackAction(params, slackConfig({ enterpriseOrgInstall: true })), + ).rejects.toThrow("unsupported_enterprise_slack_delivery"); }); function createReplyToFirstContext(hasRepliedRef: { value: boolean }) { @@ -310,6 +541,179 @@ describe("handleSlackAction", () => { }); }); + it("routes Enterprise Grid reactions through the target workspace client", async () => { + const cfg = slackConfig({ enterpriseOrgInstall: true }); + const channelId = "team:T123:channel:C123"; + + await handleSlackAction( + { + action: "react", + channelId, + messageId: "123.456", + emoji: "✅", + }, + cfg, + { + currentChannelProvider: "slack", + currentChannelId: channelId, + requesterAccountId: "default", + }, + ); + + expect(reactSlackMessage).toHaveBeenCalledWith("C123", "123.456", "✅", { + cfg, + teamId: "T123", + }); + }); + + it("qualifies a bare Enterprise Grid reaction target from the trusted current conversation", async () => { + const cfg = slackConfig({ enterpriseOrgInstall: true }); + + await handleSlackAction( + { + action: "react", + channelId: "C123", + messageId: "123.456", + emoji: "✅", + }, + cfg, + { + currentChannelProvider: "slack", + currentChannelId: "team:T123:channel:C123", + requesterAccountId: "default", + }, + ); + + expect(reactSlackMessage).toHaveBeenCalledWith("C123", "123.456", "✅", { + cfg, + teamId: "T123", + }); + }); + + it.each([ + { + name: "provider does not match", + context: { + currentChannelProvider: "discord", + currentChannelId: "team:T123:channel:C123", + requesterAccountId: "default", + }, + }, + { + name: "account does not match", + context: { + currentChannelProvider: "slack", + currentChannelId: "team:T123:channel:C123", + requesterAccountId: "other", + }, + }, + { + name: "requester account is missing", + context: { + currentChannelProvider: "slack", + currentChannelId: "team:T123:channel:C123", + }, + }, + { + name: "channel does not match", + context: { + currentChannelProvider: "slack", + currentChannelId: "team:T123:channel:C456", + requesterAccountId: "default", + }, + }, + { + name: "current target is not workspace-qualified", + context: { + currentChannelProvider: "slack", + currentChannelId: "channel:C123", + requesterAccountId: "default", + }, + }, + { + name: "current targets disagree on workspace", + context: { + currentChannelProvider: "slack", + currentChannelId: "team:T123:channel:C123", + currentMessagingTarget: "team:T456:channel:C123", + requesterAccountId: "default", + }, + }, + ])( + "rejects bare Enterprise Grid reaction targets when the trusted $name", + async ({ context }) => { + await expect( + handleSlackAction( + { + action: "react", + channelId: "C123", + messageId: "123.456", + emoji: "✅", + }, + slackConfig({ enterpriseOrgInstall: true }), + context, + ), + ).rejects.toThrow("unsupported_enterprise_slack_delivery"); + expect(reactSlackMessage).not.toHaveBeenCalled(); + }, + ); + + it("routes Enterprise Grid reaction removal through the target workspace client", async () => { + const cfg = slackConfig({ enterpriseOrgInstall: true }); + const channelId = "team:T123:channel:C123"; + + await handleSlackAction( + { + action: "react", + channelId, + messageId: "123.456", + emoji: "✅", + remove: true, + }, + cfg, + { + currentChannelProvider: "slack", + currentChannelId: channelId, + requesterAccountId: "default", + }, + ); + + expect(removeSlackReaction).toHaveBeenCalledWith("C123", "123.456", "✅", { + cfg, + teamId: "T123", + }); + }); + + it("requires a workspace-qualified reaction target for Enterprise Grid", async () => { + await expect( + handleSlackAction( + { + action: "react", + channelId: "C123", + messageId: "123.456", + emoji: "✅", + }, + slackConfig({ enterpriseOrgInstall: true }), + ), + ).rejects.toThrow("unsupported_enterprise_slack_delivery"); + expect(reactSlackMessage).not.toHaveBeenCalled(); + }); + + it("rejects workspace-qualified reaction targets for ordinary installs", async () => { + await expect( + handleSlackAction( + { + action: "react", + channelId: "team:T123:channel:C123", + messageId: "123.456", + emoji: "✅", + }, + slackConfig(), + ), + ).rejects.toThrow("unexpected_enterprise_slack_workspace"); + expect(reactSlackMessage).not.toHaveBeenCalled(); + }); + it("removes reactions on empty emoji", async () => { const cfg = slackConfig(); await handleSlackAction( @@ -638,6 +1042,65 @@ describe("handleSlackAction", () => { }); }); + it("routes Enterprise Grid uploads through a workspace-qualified destination", async () => { + const cfg = slackConfig({ enterpriseOrgInstall: true }); + + await handleSlackAction( + { + action: "uploadFile", + to: "team:T123:channel:C123", + filePath: "/tmp/report.png", + initialComment: "fresh report", + }, + cfg, + ); + + expectSlackSendCall(0, "team:T123:channel:C123", "fresh report", { + cfg, + mediaUrl: "/tmp/report.png", + threadTs: undefined, + }); + }); + + it("qualifies a bare Enterprise Grid upload destination from the trusted current conversation", async () => { + const cfg = slackConfig({ enterpriseOrgInstall: true }); + + await handleSlackAction( + { + action: "uploadFile", + to: "channel:C123", + filePath: "/tmp/report.png", + initialComment: "fresh report", + }, + cfg, + { + currentChannelProvider: "slack", + currentChannelId: "team:T123:channel:C123", + requesterAccountId: "default", + }, + ); + + expectSlackSendCall(0, "team:T123:channel:C123", "fresh report", { + cfg, + mediaUrl: "/tmp/report.png", + threadTs: undefined, + }); + }); + + it("requires a workspace-qualified upload destination for Enterprise Grid", async () => { + await expect( + handleSlackAction( + { + action: "uploadFile", + to: "channel:C123", + filePath: "/tmp/report.png", + }, + slackConfig({ enterpriseOrgInstall: true }), + ), + ).rejects.toThrow("unsupported_enterprise_slack_delivery"); + expect(sendSlackMessage).not.toHaveBeenCalled(); + }); + it.each([ { action: "sendMessage", diff --git a/extensions/slack/src/action-runtime.ts b/extensions/slack/src/action-runtime.ts index ea6108b8bfa0..c32a5177725f 100644 --- a/extensions/slack/src/action-runtime.ts +++ b/extensions/slack/src/action-runtime.ts @@ -10,6 +10,7 @@ import { normalizeOptionalLowercaseString } from "openclaw/plugin-sdk/string-coe import type { ResolvedSlackAccount } from "./accounts.js"; import { parseSlackBlocksInput } from "./blocks-input.js"; import type { SlackConversationInfo } from "./channel-type.js"; +import { assertSlackDirectSendAllowed } from "./direct-send-admission.js"; import { SLACK_TEXT_LIMIT } from "./limits.js"; import { resolveSlackChannelConfig } from "./monitor/channel-config.js"; import { isSlackChannelAllowedByPolicy } from "./monitor/policy.js"; @@ -25,6 +26,7 @@ import { type OpenClawConfig, withNormalizedTimestamp, } from "./runtime-api.js"; +import { formatSlackTarget } from "./target-parsing.js"; import { parseSlackTarget, resolveSlackChannelId, slackContextTargetsMatch } from "./targets.js"; type ConversationReadInvocationOrigin = NonNullable< @@ -80,6 +82,7 @@ export const slackActionRuntime = { cfg: OpenClawConfig; accountId?: string | null; channelId: string; + teamId?: string; operation?: "read" | "write"; requireFreshName?: boolean; }) => (await loadSlackChannelTypeRuntime()).resolveSlackConversationInfo(params), @@ -241,7 +244,7 @@ async function isSlackDmTargetConfigured(params: { function isCurrentSlackReadTarget(params: { account: ResolvedSlackAccount; - channelId: string; + target: string; context?: SlackActionContext; }): boolean { const requesterAccountId = params.context?.requesterAccountId?.trim(); @@ -250,7 +253,7 @@ function isCurrentSlackReadTarget(params: { requesterAccountId && normalizeAccountId(requesterAccountId) === normalizeAccountId(params.account.accountId) && params.context && - slackContextTargetsMatch(params.channelId, params.context), + slackContextTargetsMatch(params.target, params.context), ); } @@ -350,6 +353,7 @@ async function assertSlackReadTargetAllowed(params: { account: ResolvedSlackAccount; cfg: OpenClawConfig; channelId: string; + teamId?: string; conversationReadOrigin?: ConversationReadInvocationOrigin; context?: SlackActionContext; }) { @@ -358,7 +362,11 @@ async function assertSlackReadTargetAllowed(params: { }; const currentConversation = isCurrentSlackReadTarget({ account: params.account, - channelId: params.channelId, + target: formatSlackTarget({ + teamId: params.teamId, + kind: "channel", + id: params.channelId, + }), context: params.context, }); const directOperator = params.conversationReadOrigin === "direct-operator"; @@ -373,6 +381,7 @@ async function assertSlackReadTargetAllowed(params: { cfg: params.cfg, accountId: params.account.accountId, channelId: params.channelId, + teamId: params.teamId, operation: "read", }); if ( @@ -407,6 +416,7 @@ async function assertSlackReadTargetAllowed(params: { cfg: params.cfg, accountId: params.account.accountId, channelId: params.channelId, + teamId: params.teamId, operation: "read", ...(preliminary.shouldResolveName ? { requireFreshName: true } : {}), }); @@ -470,29 +480,97 @@ function isSlackGroupDmTargetConfigured(account: ResolvedSlackAccount, channelId }); } +type SlackActionChannelTarget = { + channelId: string; + teamId?: string; +}; + +function resolveTrustedCurrentSlackTeamId(params: { + account: ResolvedSlackAccount; + target?: NonNullable>; + context?: SlackActionContext; +}): string | undefined { + const requesterAccountId = params.context?.requesterAccountId?.trim(); + if ( + params.account.config.enterpriseOrgInstall !== true || + normalizeOptionalLowercaseString(params.context?.currentChannelProvider) !== "slack" || + !requesterAccountId || + normalizeAccountId(requesterAccountId) !== normalizeAccountId(params.account.accountId) + ) { + return undefined; + } + + const matchingTeams = new Map(); + for (const raw of [params.context?.currentChannelId, params.context?.currentMessagingTarget]) { + if (!raw) { + continue; + } + const current = parseSlackTarget(raw); + if ( + current?.teamId && + (!params.target || + (current.kind === params.target.kind && + current.id.toLowerCase() === params.target.id.toLowerCase())) + ) { + matchingTeams.set(current.teamId.toLowerCase(), current.teamId); + } + } + return matchingTeams.size === 1 ? matchingTeams.values().next().value : undefined; +} + +function resolveSlackActionTarget( + account: ResolvedSlackAccount, + raw: string, + context?: SlackActionContext, +) { + const parsed = parseSlackTarget(raw, { defaultKind: "channel" }); + if (!parsed) { + throw new Error("Slack target is required."); + } + const teamId = + parsed.teamId ?? resolveTrustedCurrentSlackTeamId({ account, target: parsed, context }); + assertSlackDirectSendAllowed(account, teamId); + return { + routingTarget: teamId ? formatSlackTarget({ teamId, kind: parsed.kind, id: parsed.id }) : raw, + teamId, + }; +} + +function resolveSlackActionChannelTarget( + account: ResolvedSlackAccount, + raw: string, + context?: SlackActionContext, +): SlackActionChannelTarget { + const resolved = resolveSlackActionTarget(account, raw, context); + const channelId = resolveSlackChannelId(raw); + return { + channelId, + teamId: resolved.teamId, + }; +} + export async function handleSlackAction( params: Record, cfg: OpenClawConfig, context?: SlackActionContext, ): Promise> { - const resolveChannelId = () => - resolveSlackChannelId( - readStringParam(params, "channelId", { - required: true, - }), - ); const action = readStringParam(params, "action", { required: true }); const accountId = readStringParam(params, "accountId"); const { resolveSlackAccount, resolveSlackOperationToken } = await loadSlackAccountsRuntime(); const account = resolveSlackAccount({ cfg, accountId }); - if (account.config.enterpriseOrgInstall === true) { - throw new Error("Slack action tools are unavailable for Enterprise Grid org installs."); - } + const resolveChannelTarget = () => + resolveSlackActionChannelTarget( + account, + readStringParam(params, "channelId", { + required: true, + }), + context, + ); const actionConfig = account.actions ?? cfg.channels?.slack?.actions; const isActionEnabled = createActionGate(actionConfig); const botToken = account.botToken?.trim(); - const buildActionOpts = (operation: "read" | "write") => { + const buildActionOpts = (operation: "read" | "write", teamId?: string) => { const token = resolveSlackOperationToken(account, operation); if (!token && account.identity === "user") { throw new Error(`Slack operation token missing for account "${account.accountId}".`); @@ -502,62 +580,58 @@ export async function handleSlackAction( cfg, ...(accountId ? { accountId } : {}), ...(tokenOverride ? { token: tokenOverride } : {}), + teamId, }; }; - const readOpts = buildActionOpts("read"); - const writeOpts = buildActionOpts("write"); - const assertReadTargetAllowed = async (channelId: string) => + const assertReadTargetAllowed = async (target: SlackActionChannelTarget) => { await assertSlackReadTargetAllowed({ account, cfg, - channelId, + channelId: target.channelId, + teamId: target.teamId, conversationReadOrigin: context?.conversationReadOrigin, context, }); + }; if (reactionsActions.has(action)) { if (!isActionEnabled("reactions")) { throw new Error("Slack reactions are disabled."); } - const channelId = resolveChannelId(); + const target = resolveChannelTarget(); + const { channelId } = target; + const readOpts = buildActionOpts("read", target.teamId); + const writeOpts = buildActionOpts("write", target.teamId); const messageId = readStringParam(params, "messageId", { required: true }); if (action === "react") { const { emoji, remove, isEmpty } = readReactionParams(params, { removeErrorMessage: "Emoji is required to remove a Slack reaction.", }); - await assertReadTargetAllowed(channelId); + await assertReadTargetAllowed(target); if (remove) { - if (writeOpts) { - await slackActionRuntime.removeSlackReaction(channelId, messageId, emoji, writeOpts); - } else { - await slackActionRuntime.removeSlackReaction(channelId, messageId, emoji); - } + await slackActionRuntime.removeSlackReaction(channelId, messageId, emoji, writeOpts); return jsonResult({ ok: true, removed: emoji }); } if (isEmpty) { - const removed = writeOpts - ? await slackActionRuntime.removeOwnSlackReactions(channelId, messageId, writeOpts) - : await slackActionRuntime.removeOwnSlackReactions(channelId, messageId); + const removed = await slackActionRuntime.removeOwnSlackReactions( + channelId, + messageId, + writeOpts, + ); return jsonResult({ ok: true, removed }); } - if (writeOpts) { - await slackActionRuntime.reactSlackMessage(channelId, messageId, emoji, writeOpts); - } else { - await slackActionRuntime.reactSlackMessage(channelId, messageId, emoji); - } + await slackActionRuntime.reactSlackMessage(channelId, messageId, emoji, writeOpts); return jsonResult({ ok: true, added: emoji }); } - await assertReadTargetAllowed(channelId); + await assertReadTargetAllowed(target); const limit = Math.min( readPositiveIntegerParam(params, "limit", { message: "limit must be a positive integer.", }) ?? SLACK_REACTION_USER_LIMIT, SLACK_REACTION_USER_LIMIT, ); - const reactions = readOpts - ? await slackActionRuntime.listSlackReactions(channelId, messageId, readOpts) - : await slackActionRuntime.listSlackReactions(channelId, messageId); + const reactions = await slackActionRuntime.listSlackReactions(channelId, messageId, readOpts); return jsonResult({ ok: true, reactions: reactions?.map((reaction) => @@ -575,6 +649,8 @@ export async function handleSlackAction( switch (action) { case "sendMessage": { const to = readStringParam(params, "to", { required: true }); + const target = resolveSlackActionTarget(account, to, context); + const destination = target.routingTarget; const content = readStringParam(params, "content", { allowEmpty: true, }); @@ -610,14 +686,14 @@ export async function handleSlackAction( } const threadTs = resolveThreadTsFromContext( readStringParam(params, "threadTs"), - to, + destination, context, { suppressImplicitThread: params.topLevel === true || params.threadTs === null, }, ); const baseSendOpts = { - ...writeOpts, + ...buildActionOpts("write"), mediaAccess: context?.mediaAccess, mediaLocalRoots: context?.mediaLocalRoots, mediaReadFile: context?.mediaReadFile, @@ -638,13 +714,13 @@ export async function handleSlackAction( // Reuse the resolved thread for both sends. Invoking the action twice // could consume replyToMode=first and move the full text off-thread. const { replyBroadcast: _replyBroadcast, ...blockSendOpts } = sendOpts; - await slackActionRuntime.sendSlackMessage(to, "", { + await slackActionRuntime.sendSlackMessage(destination, "", { ...blockSendOpts, blocks, }); - return await slackActionRuntime.sendSlackMessage(to, content, sendOpts); + return await slackActionRuntime.sendSlackMessage(destination, content, sendOpts); } - return await slackActionRuntime.sendSlackMessage(to, content ?? "", { + return await slackActionRuntime.sendSlackMessage(destination, content ?? "", { ...sendOpts, blocks, }); @@ -655,13 +731,13 @@ export async function handleSlackAction( | Awaited> | undefined; if (mediaUrl) { - lastResult = await slackActionRuntime.sendSlackMessage(to, "", { + lastResult = await slackActionRuntime.sendSlackMessage(destination, "", { ...baseSendOpts, mediaUrl, }); } for (const [index, message] of preparedMessages.entries()) { - lastResult = await slackActionRuntime.sendSlackMessage(to, message.text, { + lastResult = await slackActionRuntime.sendSlackMessage(destination, message.text, { ...baseSendOpts, ...(index === 0 && replyBroadcast ? { replyBroadcast: true } : {}), ...(message.blocks ? { blocks: message.blocks } : {}), @@ -682,14 +758,14 @@ export async function handleSlackAction( : blocks ? await (async () => { if (mediaUrl) { - await slackActionRuntime.sendSlackMessage(to, "", { + await slackActionRuntime.sendSlackMessage(destination, "", { ...sendOpts, mediaUrl, }); } return await sendContentAndBlocks(); })() - : await slackActionRuntime.sendSlackMessage(to, content ?? "", { + : await slackActionRuntime.sendSlackMessage(destination, content ?? "", { ...sendOpts, mediaUrl: mediaUrl ?? undefined, blocks, @@ -698,7 +774,7 @@ export async function handleSlackAction( // Keep "first" mode consistent even when the agent explicitly provided // threadTs: once we send a message to the current channel, consider the // first reply "used" so later tool calls don't auto-thread again. - if (context?.hasRepliedRef && slackContextTargetsMatch(to, context)) { + if (context?.hasRepliedRef && slackContextTargetsMatch(destination, context)) { context.hasRepliedRef.value = true; } @@ -706,6 +782,8 @@ export async function handleSlackAction( } case "uploadFile": { const to = readStringParam(params, "to", { required: true }); + const target = resolveSlackActionTarget(account, to, context); + const destination = target.routingTarget; const filePath = readStringParam(params, "filePath", { required: true, trim: false, @@ -723,31 +801,36 @@ export async function handleSlackAction( } const threadTs = resolveThreadTsFromContext( readStringParam(params, "threadTs"), - to, + destination, context, { suppressImplicitThread: params.topLevel === true || params.threadTs === null, }, ); - const result = await slackActionRuntime.sendSlackMessage(to, initialComment ?? "", { - ...writeOpts, - mediaUrl: filePath, - mediaAccess: context?.mediaAccess, - mediaLocalRoots: context?.mediaLocalRoots, - mediaReadFile: context?.mediaReadFile, - threadTs: threadTs ?? undefined, - ...(filename ? { uploadFileName: filename } : {}), - ...(title ? { uploadTitle: title } : {}), - }); + const result = await slackActionRuntime.sendSlackMessage( + destination, + initialComment ?? "", + { + ...buildActionOpts("write"), + mediaUrl: filePath, + mediaAccess: context?.mediaAccess, + mediaLocalRoots: context?.mediaLocalRoots, + mediaReadFile: context?.mediaReadFile, + threadTs: threadTs ?? undefined, + ...(filename ? { uploadFileName: filename } : {}), + ...(title ? { uploadTitle: title } : {}), + }, + ); - if (context?.hasRepliedRef && slackContextTargetsMatch(to, context)) { + if (context?.hasRepliedRef && slackContextTargetsMatch(destination, context)) { context.hasRepliedRef.value = true; } return jsonResult({ ok: true, result }); } case "editMessage": { - const channelId = resolveChannelId(); + const target = resolveChannelTarget(); + const { channelId } = target; const messageId = readStringParam(params, "messageId", { required: true, }); @@ -758,35 +841,30 @@ export async function handleSlackAction( if (!content && !blocks) { throw new Error("Slack editMessage requires content or blocks."); } - await assertReadTargetAllowed(channelId); - if (writeOpts) { - await slackActionRuntime.editSlackMessage(channelId, messageId, content ?? "", { - ...writeOpts, - blocks, - }); - } else { - await slackActionRuntime.editSlackMessage(channelId, messageId, content ?? "", { - blocks, - }); - } + await assertReadTargetAllowed(target); + const writeOpts = buildActionOpts("write", target.teamId); + await slackActionRuntime.editSlackMessage(channelId, messageId, content ?? "", { + ...writeOpts, + blocks, + }); return jsonResult({ ok: true }); } case "deleteMessage": { - const channelId = resolveChannelId(); + const target = resolveChannelTarget(); + const { channelId } = target; const messageId = readStringParam(params, "messageId", { required: true, }); - await assertReadTargetAllowed(channelId); - if (writeOpts) { - await slackActionRuntime.deleteSlackMessage(channelId, messageId, writeOpts); - } else { - await slackActionRuntime.deleteSlackMessage(channelId, messageId); - } + await assertReadTargetAllowed(target); + const writeOpts = buildActionOpts("write", target.teamId); + await slackActionRuntime.deleteSlackMessage(channelId, messageId, writeOpts); return jsonResult({ ok: true }); } case "readMessages": { - const channelId = resolveChannelId(); - await assertReadTargetAllowed(channelId); + const target = resolveChannelTarget(); + const { channelId } = target; + await assertReadTargetAllowed(target); + const readOpts = buildActionOpts("read", target.teamId); const limit = readPositiveIntegerParam(params, "limit", { message: "limit must be a positive integer.", }); @@ -827,16 +905,18 @@ export async function handleSlackAction( "Slack file download requires channelId or to so the read target can be authorized.", ); } - const channelId = resolveSlackChannelId(channelTarget); - await assertReadTargetAllowed(channelId); + const target = resolveSlackActionChannelTarget(account, channelTarget, context); + const { channelId } = target; + await assertReadTargetAllowed(target); const threadId = readStringParam(params, "threadId") ?? readStringParam(params, "replyTo"); const maxBytes = account.config?.mediaMaxMb ? account.config.mediaMaxMb * 1024 * 1024 : 20 * 1024 * 1024; const readToken = resolveSlackOperationToken(account, "read"); + const readOpts = buildActionOpts("read", target.teamId); const downloaded = await slackActionRuntime.downloadSlackFile(fileId, { ...readOpts, - ...(readToken && !readOpts?.token ? { token: readToken } : {}), + ...(readToken && !readOpts.token ? { token: readToken } : {}), maxBytes, channelId, threadId: threadId ?? undefined, @@ -882,35 +962,28 @@ export async function handleSlackAction( if (!isActionEnabled("pins")) { throw new Error("Slack pins are disabled."); } - const channelId = resolveChannelId(); + const target = resolveChannelTarget(); + const { channelId } = target; + const readOpts = buildActionOpts("read", target.teamId); + const writeOpts = buildActionOpts("write", target.teamId); if (action === "pinMessage") { const messageId = readStringParam(params, "messageId", { required: true, }); - await assertReadTargetAllowed(channelId); - if (writeOpts) { - await slackActionRuntime.pinSlackMessage(channelId, messageId, writeOpts); - } else { - await slackActionRuntime.pinSlackMessage(channelId, messageId); - } + await assertReadTargetAllowed(target); + await slackActionRuntime.pinSlackMessage(channelId, messageId, writeOpts); return jsonResult({ ok: true }); } if (action === "unpinMessage") { const messageId = readStringParam(params, "messageId", { required: true, }); - await assertReadTargetAllowed(channelId); - if (writeOpts) { - await slackActionRuntime.unpinSlackMessage(channelId, messageId, writeOpts); - } else { - await slackActionRuntime.unpinSlackMessage(channelId, messageId); - } + await assertReadTargetAllowed(target); + await slackActionRuntime.unpinSlackMessage(channelId, messageId, writeOpts); return jsonResult({ ok: true }); } - await assertReadTargetAllowed(channelId); - const pins = writeOpts - ? await slackActionRuntime.listSlackPins(channelId, readOpts) - : await slackActionRuntime.listSlackPins(channelId); + await assertReadTargetAllowed(target); + const pins = await slackActionRuntime.listSlackPins(channelId, readOpts); const normalizedPins = pins.map((pin) => { const message = pin.message ? withNormalizedTimestamp( @@ -929,9 +1002,14 @@ export async function handleSlackAction( } const userId = readStringParam(params, "userId", { required: true }); assertSlackMemberInfoAllowed({ account, context, userId }); - const info = readOpts - ? await slackActionRuntime.getSlackMemberInfo(userId, readOpts) - : await slackActionRuntime.getSlackMemberInfo(userId); + const teamId = resolveTrustedCurrentSlackTeamId({ account, context }); + if (account.config.enterpriseOrgInstall === true) { + assertSlackDirectSendAllowed(account, teamId); + } + const info = await slackActionRuntime.getSlackMemberInfo( + userId, + buildActionOpts("read", teamId), + ); return jsonResult({ ok: true, info }); } @@ -942,9 +1020,11 @@ export async function handleSlackAction( const limit = readPositiveIntegerParam(params, "limit", { message: "limit must be a positive integer.", }); - const result = readOpts - ? await slackActionRuntime.listSlackEmojis(readOpts) - : await slackActionRuntime.listSlackEmojis(); + const teamId = resolveTrustedCurrentSlackTeamId({ account, context }); + if (account.config.enterpriseOrgInstall === true) { + assertSlackDirectSendAllowed(account, teamId); + } + const result = await slackActionRuntime.listSlackEmojis(buildActionOpts("read", teamId)); if (limit != null && limit > 0 && result.emoji != null) { const entries = Object.entries(result.emoji).toSorted(([a], [b]) => a.localeCompare(b)); if (entries.length > limit) { diff --git a/extensions/slack/src/actions.download-file.test.ts b/extensions/slack/src/actions.download-file.test.ts index f07a8dd1f481..12f0eb55fda3 100644 --- a/extensions/slack/src/actions.download-file.test.ts +++ b/extensions/slack/src/actions.download-file.test.ts @@ -267,7 +267,9 @@ describe("downloadSlackFile", () => { maxBytes: 1024, }); - expect(createSlackLookupClientMock).toHaveBeenCalledWith("xoxb-from-cfg"); + expect(createSlackLookupClientMock).toHaveBeenCalledWith("xoxb-from-cfg", { + teamId: undefined, + }); expect(resolveSlackMedia).toHaveBeenCalledWith({ files: [ { diff --git a/extensions/slack/src/actions.reactions.test.ts b/extensions/slack/src/actions.reactions.test.ts index 0dd058cef42a..e7f9bba3d280 100644 --- a/extensions/slack/src/actions.reactions.test.ts +++ b/extensions/slack/src/actions.reactions.test.ts @@ -1,8 +1,15 @@ // Slack tests cover actions.reactions plugin behavior. import type { WebClient } from "@slack/web-api"; -import { describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; import { reactSlackMessage, removeOwnSlackReactions, removeSlackReaction } from "./actions.js"; +const getSlackWriteClientMock = vi.hoisted(() => vi.fn()); + +vi.mock("./client.js", async () => { + const actual = await vi.importActual("./client.js"); + return { ...actual, getSlackWriteClient: getSlackWriteClientMock }; +}); + function createClient() { return { auth: { @@ -39,6 +46,27 @@ function slackPlatformError(error: string) { } describe("reactSlackMessage", () => { + beforeEach(() => { + getSlackWriteClientMock.mockReset(); + }); + + it("uses a workspace-scoped write client for Enterprise Grid reactions", async () => { + const client = createClient(); + getSlackWriteClientMock.mockReturnValue(client); + + await reactSlackMessage("C1", "123.456", "✅", { + teamId: "T1", + token: "xoxb-test", + }); + + expect(getSlackWriteClientMock).toHaveBeenCalledWith("xoxb-test", { teamId: "T1" }); + expect(client.reactions.add).toHaveBeenCalledWith({ + channel: "C1", + timestamp: "123.456", + name: "white_check_mark", + }); + }); + it("treats already_reacted as idempotent success", async () => { const client = createClient(); client.reactions.add.mockRejectedValueOnce(slackPlatformError("already_reacted")); diff --git a/extensions/slack/src/actions.read.test.ts b/extensions/slack/src/actions.read.test.ts index 75a722ec7d0f..b0fb847fa7a0 100644 --- a/extensions/slack/src/actions.read.test.ts +++ b/extensions/slack/src/actions.read.test.ts @@ -411,6 +411,8 @@ describe("Slack read actions", () => { cfg: { channels: { slack: { enabled: true, botToken: "test-auth-token" } } }, } as Parameters[1]); - expect(createSlackLookupClientMock).toHaveBeenCalledWith("test-auth-token"); + expect(createSlackLookupClientMock).toHaveBeenCalledWith("test-auth-token", { + teamId: undefined, + }); }); }); diff --git a/extensions/slack/src/actions.ts b/extensions/slack/src/actions.ts index 136daae87e53..fccc1e1978b8 100644 --- a/extensions/slack/src/actions.ts +++ b/extensions/slack/src/actions.ts @@ -32,6 +32,7 @@ export type SlackActionClientOpts = { cfg?: OpenClawConfig; accountId?: string; token?: string; + teamId?: string; client?: WebClient; }; @@ -214,7 +215,10 @@ async function getClient(opts: SlackActionClientOpts = {}, mode: "read" | "write return opts.client; } const token = resolveToken(opts.token, opts.accountId, opts.cfg); - return mode === "write" ? getSlackWriteClient(token) : createSlackLookupClient(token); + if (mode === "write") { + return getSlackWriteClient(token, { teamId: opts.teamId }); + } + return createSlackLookupClient(token, { teamId: opts.teamId }); } async function resolveBotUserId(client: WebClient) { diff --git a/extensions/slack/src/approval-native-gates.ts b/extensions/slack/src/approval-native-gates.ts index 0f18944b5144..cfabac24ad86 100644 --- a/extensions/slack/src/approval-native-gates.ts +++ b/extensions/slack/src/approval-native-gates.ts @@ -34,7 +34,12 @@ import { getSlackExecApprovalApprovers, isSlackExecApprovalClientEnabled, } from "./exec-approvals.js"; -import { canonicalizeSlackApiTargetId, parseSlackTarget } from "./target-parsing.js"; +import { + canonicalizeSlackApiTargetId, + formatSlackTarget, + parseSlackTarget, + type SlackTarget, +} from "./target-parsing.js"; export type SlackApprovalKind = "exec" | "plugin"; export type SlackNativeApprovalRequest = ExecApprovalRequest | PluginApprovalRequest; @@ -131,7 +136,7 @@ export function resolveTurnSourceSlackOriginTarget( return null; } return { - to: `${parsed.kind}:${parsed.id}`, + to: formatSlackApprovalTarget(parsed), threadId: stringifyRouteThreadId(request.request.turnSourceThreadId), }; } @@ -164,7 +169,7 @@ export function resolveSlackFallbackOriginTarget( return null; } return { - to: `${parsed.kind}:${canonicalizeSlackApiTargetId(parsed.kind, parsed.id)}`, + to: formatSlackApprovalTarget(parsed, canonicalizeSlackApiTargetId(parsed.kind, parsed.id)), threadId: sessionTarget.threadId, }; } @@ -232,7 +237,7 @@ export function normalizeSlackForwardTarget( return null; } return { - to: `${parsed.kind}:${parsed.id}`, + to: formatSlackApprovalTarget(parsed), accountId: normalizeOptionalString(target.accountId), threadId: stringifyRouteThreadId(target.threadId), }; @@ -440,3 +445,9 @@ export function shouldHandleSlackNativeApprovalRequest(params: { sessionFilter: config?.sessionFilter, }); } + +function formatSlackApprovalTarget(target: SlackTarget, id = target.id): string { + return target.teamId + ? formatSlackTarget({ teamId: target.teamId, kind: target.kind, id }) + : `${target.kind}:${id}`; +} diff --git a/extensions/slack/src/approval-native.test.ts b/extensions/slack/src/approval-native.test.ts index 84222fb1bce7..1c546c6722ad 100644 --- a/extensions/slack/src/approval-native.test.ts +++ b/extensions/slack/src/approval-native.test.ts @@ -767,7 +767,7 @@ describe("slack native approval adapter", () => { ); expect(target).toEqual({ - to: "channel:team:T123:channel:C08GQH53EJM", + to: "team:T123:channel:C08GQH53EJM", threadId: undefined, }); }); diff --git a/extensions/slack/src/channel-actions.ts b/extensions/slack/src/channel-actions.ts index abe26f43e08f..ce56619d27a9 100644 --- a/extensions/slack/src/channel-actions.ts +++ b/extensions/slack/src/channel-actions.ts @@ -9,7 +9,7 @@ import type { SlackActionContext } from "./action-runtime.js"; import { handleSlackMessageAction } from "./message-action-dispatch.js"; import { extractSlackToolSend } from "./message-actions.js"; import { describeSlackMessageTool } from "./message-tool-api.js"; -import { resolveSlackChannelId } from "./targets.js"; +import { formatSlackTarget, parseSlackTarget, resolveSlackChannelId } from "./target-parsing.js"; type SlackActionInvoke = ( action: Record, @@ -66,12 +66,15 @@ export function createSlackActions( extractToolSend: ({ args }) => extractSlackToolSend(args), isToolDeliveryAction: ({ args }) => typeof args.action === "string" && SLACK_TOOL_DELIVERY_ACTIONS.has(args.action), - prepareSendPayload: ({ ctx, payload }) => (ctx.action === "send" ? payload : null), + prepareSendPayload: ({ ctx, to, payload }) => + ctx.action === "send" && !shouldUseWorkspaceAwareSlackActionSend(to, ctx.toolContext) + ? payload + : null, handleAction: async (ctx) => { return await handleSlackMessageAction({ providerId, ctx, - normalizeChannelId: resolveSlackChannelId, + normalizeChannelId: normalizeSlackActionChannelTarget, includeReadThreadId: true, invoke: async (action, cfg, toolContext) => { const actionContext = resolveSlackActionContext(ctx, toolContext); @@ -83,3 +86,33 @@ export function createSlackActions( }, }; } + +function normalizeSlackActionChannelTarget(raw: string): string { + const target = parseSlackTarget(raw, { defaultKind: "channel" }); + const channelId = resolveSlackChannelId(raw); + return formatSlackTarget({ teamId: target?.teamId, kind: "channel", id: channelId }); +} + +function shouldUseWorkspaceAwareSlackActionSend( + rawTarget: string, + context: ChannelMessageActionContext["toolContext"], +): boolean { + const target = parseSlackTarget(rawTarget, { defaultKind: "channel" }); + if (!target || target.teamId) { + return false; + } + for (const rawCurrentTarget of [context?.currentChannelId, context?.currentMessagingTarget]) { + if (!rawCurrentTarget) { + continue; + } + const currentTarget = parseSlackTarget(rawCurrentTarget); + if ( + currentTarget?.teamId && + currentTarget.kind === target.kind && + currentTarget.id.toLowerCase() === target.id.toLowerCase() + ) { + return true; + } + } + return false; +} diff --git a/extensions/slack/src/channel-type.test.ts b/extensions/slack/src/channel-type.test.ts index 291f9bbeae06..24853f800b17 100644 --- a/extensions/slack/src/channel-type.test.ts +++ b/extensions/slack/src/channel-type.test.ts @@ -117,7 +117,7 @@ describe("resolveSlackChannelType", () => { type: "dm", user: "U09G2DJ0275", }); - expect(createSlackReadClientMock).toHaveBeenCalledWith("xoxb-test"); + expect(createSlackReadClientMock).toHaveBeenCalledWith("xoxb-test", { teamId: undefined }); expect(createSlackWebClientMock).not.toHaveBeenCalled(); expect(conversationsInfoMock).toHaveBeenCalledWith({ channel: "D0AEWSDHAQH" }); expect(conversationsOpenMock).not.toHaveBeenCalled(); @@ -149,7 +149,7 @@ describe("resolveSlackChannelType", () => { type: "dm", user: "U09G2DJ0275", }); - expect(createSlackWebClientMock).toHaveBeenCalledWith("botB"); + expect(createSlackWebClientMock).toHaveBeenCalledWith("botB", { teamId: undefined }); expect(createSlackReadClientMock).not.toHaveBeenCalled(); expect(conversationsOpenMock).toHaveBeenCalledWith({ channel: "D0AEWSDHAQH", @@ -185,7 +185,9 @@ describe("resolveSlackChannelType", () => { type: "dm", user: "U09G2DJ0275", }); - expect(createSlackWebClientMock).toHaveBeenCalledWith("test-user-token"); + expect(createSlackWebClientMock).toHaveBeenCalledWith("test-user-token", { + teamId: undefined, + }); expect(createSlackReadClientMock).not.toHaveBeenCalled(); expect(conversationsOpenMock).toHaveBeenCalledWith({ channel: "D0AEWSDHAQH", @@ -221,7 +223,7 @@ describe("resolveSlackChannelType", () => { type: "dm", user: "U09G2DJ0275", }); - expect(createSlackReadClientMock).toHaveBeenCalledWith("envUsr"); + expect(createSlackReadClientMock).toHaveBeenCalledWith("envUsr", { teamId: undefined }); expect(createSlackWebClientMock).not.toHaveBeenCalled(); expect(conversationsInfoMock).toHaveBeenCalledWith({ channel: "D0AEWSDHAQH" }); expect(conversationsOpenMock).not.toHaveBeenCalled(); @@ -253,7 +255,7 @@ describe("resolveSlackChannelType", () => { type: "dm", user: "U09G2DJ0275", }); - expect(createSlackWebClientMock).toHaveBeenCalledWith("envBot"); + expect(createSlackWebClientMock).toHaveBeenCalledWith("envBot", { teamId: undefined }); expect(createSlackReadClientMock).not.toHaveBeenCalled(); expect(conversationsOpenMock).toHaveBeenCalledWith({ channel: "D0AEWSDHAQH", @@ -289,7 +291,9 @@ describe("resolveSlackChannelType", () => { type: "group", name: "mpdm-alice--bob-1", }); - expect(createSlackReadClientMock).toHaveBeenCalledWith("xoxp-reader"); + expect(createSlackReadClientMock).toHaveBeenCalledWith("xoxp-reader", { + teamId: undefined, + }); expect(createSlackWebClientMock).not.toHaveBeenCalled(); expect(conversationsInfoMock).toHaveBeenCalledWith({ channel: "C0MPIM" }); }); @@ -334,8 +338,12 @@ describe("resolveSlackChannelType", () => { }), ).resolves.toMatchObject({ name: "after-rotation" }); - expect(createSlackReadClientMock).toHaveBeenNthCalledWith(1, "xoxb-before"); - expect(createSlackReadClientMock).toHaveBeenNthCalledWith(2, "xoxb-after"); + expect(createSlackReadClientMock).toHaveBeenNthCalledWith(1, "xoxb-before", { + teamId: undefined, + }); + expect(createSlackReadClientMock).toHaveBeenNthCalledWith(2, "xoxb-after", { + teamId: undefined, + }); expect(conversationsInfoMock).toHaveBeenCalledTimes(2); }); diff --git a/extensions/slack/src/channel-type.ts b/extensions/slack/src/channel-type.ts index 9e2413d793a5..7674f1828ce2 100644 --- a/extensions/slack/src/channel-type.ts +++ b/extensions/slack/src/channel-type.ts @@ -74,6 +74,7 @@ export async function resolveSlackConversationInfo(params: { cfg: OpenClawConfig; accountId?: string | null; channelId: string; + teamId?: string; operation?: "read" | "write"; requireFreshName?: boolean; }): Promise { @@ -87,7 +88,8 @@ export async function resolveSlackConversationInfo(params: { const userToken = normalizeOptionalString(account.userToken); const credentialRole = token ? (token === userToken ? "user" : "bot") : "none"; const credentialFingerprint = token ? fingerprintSlackCredential(token) : "none"; - const cacheKey = `${account.accountId}:${operation}:${credentialRole}:${credentialFingerprint}:${channelId}`; + const teamId = normalizeLowercaseStringOrEmpty(params.teamId) || "no-team-id"; + const cacheKey = `${account.accountId}:${teamId}:${operation}:${credentialRole}:${credentialFingerprint}:${channelId}`; if (!params.requireFreshName) { const cached = getCachedSlackConversationInfo(cacheKey); if (cached) { @@ -101,7 +103,7 @@ export async function resolveSlackConversationInfo(params: { // Read-only classification stays on conversations.info. conversations.open is // write-scoped and must only run when the caller explicitly requests a write. if (isNativeImChannel && operation === "write") { - const client = createSlackWebClient(token); + const client = createSlackWebClient(token, { teamId: params.teamId }); const opened = await client.conversations.open({ channel: channelId, prevent_creation: true, @@ -117,7 +119,7 @@ export async function resolveSlackConversationInfo(params: { } return result; } - const client = createSlackReadClient(token); + const client = createSlackReadClient(token, { teamId: params.teamId }); const info = await client.conversations.info({ channel: channelId }); const channel = info.channel as | { is_im?: boolean; is_mpim?: boolean; name?: string; user?: string } @@ -151,6 +153,7 @@ export async function resolveSlackChannelType(params: { cfg: OpenClawConfig; accountId?: string | null; channelId: string; + teamId?: string; }): Promise<"channel" | "group" | "dm" | "unknown"> { return (await resolveSlackConversationInfo(params)).type; } diff --git a/extensions/slack/src/channel.test.ts b/extensions/slack/src/channel.test.ts index 90fb12976d62..66aff2d73f85 100644 --- a/extensions/slack/src/channel.test.ts +++ b/extensions/slack/src/channel.test.ts @@ -15,13 +15,17 @@ const { resolveSlackDmChannelIdMock, sendMessageSlackMock } = vi.hoisted(() => ( resolveSlackDmChannelIdMock: vi.fn(), sendMessageSlackMock: vi.fn(), })); -const { assistantThreadsSetStatusMock, conversationsInfoMock, conversationsOpenMock } = vi.hoisted( - () => ({ - assistantThreadsSetStatusMock: vi.fn(), - conversationsInfoMock: vi.fn(), - conversationsOpenMock: vi.fn(), - }), -); +const { + assistantThreadsSetStatusMock, + conversationsInfoMock, + conversationsOpenMock, + getSlackWriteClientMock, +} = vi.hoisted(() => ({ + assistantThreadsSetStatusMock: vi.fn(), + conversationsInfoMock: vi.fn(), + conversationsOpenMock: vi.fn(), + getSlackWriteClientMock: vi.fn(), +})); vi.mock("./action-runtime.js", async () => { const actual = await vi.importActual("./action-runtime.js"); @@ -52,7 +56,7 @@ vi.mock("./client.js", async () => { return { ...actual, createSlackReadClient: vi.fn(createClient), - createSlackWebClient: vi.fn(createClient), + getSlackWriteClient: getSlackWriteClientMock.mockImplementation(createClient), }; }); @@ -66,6 +70,7 @@ beforeEach(async () => { assistantThreadsSetStatusMock.mockResolvedValue({ ok: true }); conversationsInfoMock.mockReset(); conversationsOpenMock.mockReset(); + getSlackWriteClientMock.mockClear(); setSlackRuntime({ channel: { slack: { @@ -96,6 +101,14 @@ function requireSlackHandleAction() { return handleAction; } +function requireSlackPrepareSendPayload() { + const prepareSendPayload = slackPlugin.actions?.prepareSendPayload; + if (!prepareSendPayload) { + throw new Error("slack actions.prepareSendPayload unavailable"); + } + return prepareSendPayload; +} + function requireSlackSendText() { const sendText = slackPlugin.outbound?.sendText; if (!sendText) { @@ -202,6 +215,45 @@ function findSchemaEntry( } describe("slackPlugin actions", () => { + it("keeps a bare current Grid send on the workspace-aware Slack action path", async () => { + const prepareSendPayload = requireSlackPrepareSendPayload(); + const payload = { text: "hello" }; + + const prepared = await prepareSendPayload({ + ctx: { + action: "send", + channel: "slack", + cfg: {}, + params: {}, + toolContext: { + currentChannelId: "team:T123:channel:C123", + currentChannelProvider: "slack", + }, + }, + to: "channel:C123", + payload, + } as never); + + expect(prepared).toBeNull(); + }); + + it("keeps qualified and cross-channel sends on the core Slack delivery path", async () => { + const prepareSendPayload = requireSlackPrepareSendPayload(); + const payload = { text: "hello" }; + const ctx = { + action: "send", + channel: "slack", + cfg: {}, + params: {}, + toolContext: { currentChannelId: "team:T123:channel:C123" }, + }; + + expect(prepareSendPayload({ ctx, to: "team:T123:channel:C123", payload } as never)).toBe( + payload, + ); + expect(prepareSendPayload({ ctx, to: "channel:C999", payload } as never)).toBe(payload); + }); + it("prefers session lookup for announce target routing", () => { expect(slackPlugin.meta.preferSessionLookupForAnnounceTarget).toBe(true); }); @@ -682,6 +734,34 @@ describe("slackPlugin status", () => { }); }); + it("matches the workspace-qualified session identity produced by Enterprise ingress", async () => { + const resolveRoute = slackPlugin.messaging?.resolveOutboundSessionRoute; + if (!resolveRoute) { + throw new Error("slack messaging.resolveOutboundSessionRoute unavailable"); + } + + const channelRoute = await resolveRoute({ + cfg: {} as OpenClawConfig, + agentId: "main", + target: "team:T123:channel:C456", + }); + const dmRoute = await resolveRoute({ + cfg: {} as OpenClawConfig, + agentId: "main", + accountId: "default", + target: "team:T123:user:U456", + }); + + expectRecordFields(channelRoute, "Enterprise Slack channel route", { + baseSessionKey: "agent:main:slack:channel:team:t123:channel:c456", + to: "team:T123:channel:C456", + }); + expectRecordFields(dmRoute, "Enterprise Slack DM route", { + baseSessionKey: "agent:main:main:account:default:team:t123", + to: "team:T123:user:U456", + }); + }); + it("routes a folded bare W user id as a direct session", async () => { const resolveRoute = slackPlugin.messaging?.resolveOutboundSessionRoute; if (!resolveRoute) { @@ -1009,6 +1089,62 @@ describe("slackPlugin outbound", () => { expect(sendSlack).not.toHaveBeenCalled(); }); + it("sends Enterprise messages when the existing target carries the workspace", async () => { + const sendSlack = vi.fn().mockResolvedValue({ messageId: "m-enterprise" }); + const sendText = requireSlackSendText(); + + const result = await sendText({ + cfg: { channels: { slack: { enterpriseOrgInstall: true } } }, + to: "team:T123:channel:C456", + text: "hello", + accountId: "default", + deps: { sendSlack }, + }); + + expect(requireMockCallArgValue(sendSlack, 0, 0)).toBe("team:T123:channel:C456"); + expect(result).toEqual({ channel: "slack", messageId: "m-enterprise" }); + }); + + it("rejects workspace-qualified targets for ordinary Slack installations", async () => { + const sendSlack = vi.fn().mockResolvedValue({ messageId: "should-not-send" }); + + await expect( + requireSlackSendText()({ + cfg, + to: "team:T123:channel:C456", + text: "hello", + accountId: "default", + deps: { sendSlack }, + }), + ).rejects.toThrow("unexpected_enterprise_slack_workspace"); + expect(sendSlack).not.toHaveBeenCalled(); + }); + + it("admits deferred Enterprise messages only when their target carries the workspace", () => { + const admit = slackPlugin.message?.durableFinal?.admitDeferredDelivery; + if (!admit) { + throw new Error("slack deferred-delivery admission unavailable"); + } + const enterpriseCfg = { + channels: { slack: { enterpriseOrgInstall: true } }, + } as OpenClawConfig; + const base = { + cfg: enterpriseCfg, + accountId: "default", + kind: "text" as const, + queueId: "q1", + payloads: [{ text: "hello" }], + }; + + expect(admit({ ...base, to: "team:T123:channel:C456" } as never)).toEqual({ + status: "allowed", + }); + expect(admit({ ...base, to: "channel:C456" } as never)).toEqual({ + status: "permanent_rejection", + reason: "unsupported_enterprise_slack_delivery", + }); + }); + it("forwards agent identity through the registered text sender", async () => { const sendText = requireSlackSendText(); @@ -1145,6 +1281,21 @@ describe("slackPlugin outbound", () => { }); }); + it("uses the workspace-partitioned write-client cache for Grid assistant status", async () => { + const target = { + cfg: { channels: { slack: { botToken: "xoxb-test", enterpriseOrgInstall: true } } }, + to: "team:T123:channel:C456", + accountId: "default", + threadId: "1712345678.123456", + }; + + await requireSlackHeartbeatSendTyping()(target); + await requireSlackHeartbeatClearTyping()(target); + + expect(getSlackWriteClientMock).toHaveBeenNthCalledWith(1, "xoxb-test", { teamId: "T123" }); + expect(getSlackWriteClientMock).toHaveBeenNthCalledWith(2, "xoxb-test", { teamId: "T123" }); + }); + it("resolves user targets to concrete DM channels for assistant status", async () => { await requireSlackHeartbeatSendTyping()({ cfg, @@ -1363,6 +1514,27 @@ describe("slackPlugin outbound", () => { expect(result).toEqual({ channel: "slack", messageId: "m-media-local" }); }); + it("preserves workspace-qualified Enterprise media delivery", async () => { + const sendSlack = vi.fn().mockResolvedValue({ messageId: "m-grid-media" }); + const sendMedia = requireSlackSendMedia(); + + const result = await sendMedia({ + cfg: { channels: { slack: { enterpriseOrgInstall: true } } }, + to: "team:T123:channel:C999", + text: "attachment", + mediaUrl: "/tmp/workspace/report.txt", + mediaLocalRoots: ["/tmp/workspace"], + accountId: "default", + deps: { sendSlack }, + }); + + expect(requireMockCallArgValue(sendSlack, 0, 0)).toBe("team:T123:channel:C999"); + expectRecordFields(requireMockCallArg(sendSlack, 0, 2), "send options", { + mediaUrl: "/tmp/workspace/report.txt", + }); + expect(result).toEqual({ channel: "slack", messageId: "m-grid-media" }); + }); + it("sends block payload media first, then the final block message", async () => { const sendSlack = vi .fn() diff --git a/extensions/slack/src/channel.ts b/extensions/slack/src/channel.ts index 20015fd94538..77834d8bc943 100644 --- a/extensions/slack/src/channel.ts +++ b/extensions/slack/src/channel.ts @@ -56,7 +56,7 @@ import { type OpenClawConfig, } from "./channel-api.js"; import { resolveSlackChannelType, resolveSlackConversationInfo } from "./channel-type.js"; -import { createSlackWebClient } from "./client.js"; +import { getSlackWriteClient } from "./client.js"; import { assertSlackDirectSendAllowed } from "./direct-send-admission.js"; import { formatSlackError } from "./errors.js"; import { shouldSuppressLocalSlackExecApprovalPrompt } from "./exec-approvals.js"; @@ -74,7 +74,11 @@ import { SLACK_CHANNEL, slackConfigAdapter, } from "./shared.js"; -import { canonicalizeSlackApiTargetId, parseSlackTarget } from "./target-parsing.js"; +import { + canonicalizeSlackApiTargetId, + formatSlackTarget, + parseSlackTarget, +} from "./target-parsing.js"; import { slackContextTargetsMatch } from "./targets.js"; import { normalizeSlackThreadTsCandidate, resolveSlackThreadTsValue } from "./thread-ts.js"; import { buildSlackThreadingToolContext } from "./threading-tool-context.js"; @@ -182,6 +186,7 @@ const loadSlackDirectoryLiveModule = createLazyRuntimeModule(() => import("./dir async function resolveSlackSendContext(params: { cfg: Parameters[0]["cfg"]; accountId?: string; + to: string; deps?: { [channelId: string]: unknown }; replyToId?: string | number | null; threadId?: string | number | null; @@ -190,7 +195,9 @@ async function resolveSlackSendContext(params: { // expected to be resolved from this snapshot. Strict mode // is intentional so boot-time misconfigurations surface loudly. See #68237. const account = resolveSlackAccount({ cfg: params.cfg, accountId: params.accountId }); - assertSlackDirectSendAllowed(account); + const target = parseSlackTarget(params.to, { defaultKind: "channel" }); + const teamId = target?.teamId; + assertSlackDirectSendAllowed(account, teamId); const send = resolveOutboundSendDep(params.deps, "slack") ?? (await loadSlackSendRuntime()).sendMessageSlack; @@ -198,7 +205,7 @@ async function resolveSlackSendContext(params: { const botToken = account.botToken?.trim(); const tokenOverride = token && token !== botToken ? token : undefined; const threadTsValue = resolveSlackThreadTsValue(params); - return { send, threadTsValue, tokenOverride }; + return { send, threadTsValue, tokenOverride, to: params.to }; } async function setSlackHeartbeatThreadStatus(params: { @@ -214,13 +221,13 @@ async function setSlackHeartbeatThreadStatus(params: { return; } const account = resolveSlackAccount({ cfg: params.cfg, accountId: params.accountId }); - assertSlackDirectSendAllowed(account); + assertSlackDirectSendAllowed(account, target.teamId); const botToken = normalizeOptionalString(account.botToken); if (!botToken) { return; } try { - const client = createSlackWebClient(botToken); + const client = getSlackWriteClient(botToken, { teamId: target.teamId }); const apiTargetId = canonicalizeSlackApiTargetId(target.kind, target.id, params.to); const channelId = target.kind === "channel" @@ -275,7 +282,7 @@ function resolveSlackRouteTarget(raw: string) { return null; } return { - to: target.id, + to: target.teamId ? target.normalized : target.id, chatType: target.kind === "user" ? ("direct" as const) : ("channel" as const), }; } @@ -357,7 +364,7 @@ async function resolveSlackOutboundSessionRoute(params: { 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; + let peerId = formatSlackTarget(parsed); let recipientSessionExact = isDm ? /^[UW][A-Z0-9]{8,}$/i.test(parsed.id) : /^C[A-Z0-9]{8,}$/i.test(parsed.id); @@ -366,18 +373,24 @@ async function resolveSlackOutboundSessionRoute(params: { cfg: params.cfg, accountId: params.accountId, channelId: apiTargetId, + teamId: parsed.teamId, }); if (conversation.type !== "dm" || !conversation.user) { return null; } peerKind = "direct"; - peerId = conversation.user; + peerId = formatSlackTarget({ + teamId: parsed.teamId, + kind: "user", + id: conversation.user, + }); recipientSessionExact = true; } else if (!isDm && /^G/i.test(parsed.id)) { const channelType = await resolveSlackChannelType({ cfg: params.cfg, accountId: params.accountId, channelId: apiTargetId, + teamId: parsed.teamId, }); if (channelType === "group") { peerKind = "group"; @@ -391,12 +404,18 @@ async function resolveSlackOutboundSessionRoute(params: { kind: peerKind, id: peerId, }; - const baseSessionKey = buildSlackBaseSessionKey({ + const unpartitionedBaseSessionKey = buildSlackBaseSessionKey({ cfg: params.cfg, agentId: params.agentId, accountId: params.accountId, peer, }); + const baseSessionKey = + parsed.teamId && peerKind === "direct" && (params.cfg.session?.dmScope ?? "main") === "main" + ? `${unpartitionedBaseSessionKey}:account:${encodeURIComponent( + resolveSlackAccount({ cfg: params.cfg, accountId: params.accountId }).accountId, + ).toLowerCase()}:team:${encodeURIComponent(parsed.teamId).toLowerCase()}` + : unpartitionedBaseSessionKey; return buildThreadAwareOutboundSessionRoute({ route: { sessionKey: baseSessionKey, @@ -410,7 +429,7 @@ async function resolveSlackOutboundSessionRoute(params: { : peerKind === "group" ? `slack:group:${peerId}` : `slack:channel:${peerId}`, - to: peerKind === "direct" ? `user:${peerId}` : `channel:${peerId}`, + to: parsed.teamId ? peerId : peerKind === "direct" ? `user:${peerId}` : `channel:${peerId}`, }, replyToId: params.replyToId, threadId: params.threadId, @@ -498,9 +517,10 @@ const slackChannelOutbound: ChannelOutboundAdapter = { }, }), sendPayload: async (ctx) => { - const { send, threadTsValue, tokenOverride } = await resolveSlackSendContext({ + const { send, threadTsValue, tokenOverride, to } = await resolveSlackSendContext({ cfg: ctx.cfg, accountId: ctx.accountId ?? undefined, + to: ctx.to, deps: ctx.deps, replyToId: ctx.replyToId, threadId: ctx.threadId, @@ -508,6 +528,7 @@ const slackChannelOutbound: ChannelOutboundAdapter = { const { slackOutbound } = await loadSlackOutboundAdapterModule(); return await slackOutbound.sendPayload!({ ...ctx, + to, replyToId: threadTsValue, threadId: null, deliveryQueueId: undefined, @@ -520,9 +541,10 @@ const slackChannelOutbound: ChannelOutboundAdapter = { }); }, sendText: async (ctx) => { - const { send, threadTsValue, tokenOverride } = await resolveSlackSendContext({ + const { send, threadTsValue, tokenOverride, to } = await resolveSlackSendContext({ cfg: ctx.cfg, accountId: ctx.accountId ?? undefined, + to: ctx.to, deps: ctx.deps, replyToId: ctx.replyToId, threadId: ctx.threadId, @@ -530,6 +552,7 @@ const slackChannelOutbound: ChannelOutboundAdapter = { const { slackOutbound } = await loadSlackOutboundAdapterModule(); return await slackOutbound.sendText!({ ...ctx, + to, replyToId: threadTsValue, threadId: null, deliveryQueueId: undefined, @@ -544,9 +567,10 @@ const slackChannelOutbound: ChannelOutboundAdapter = { }); }, sendMedia: async (ctx) => { - const { send, threadTsValue, tokenOverride } = await resolveSlackSendContext({ + const { send, threadTsValue, tokenOverride, to } = await resolveSlackSendContext({ cfg: ctx.cfg, accountId: ctx.accountId ?? undefined, + to: ctx.to, deps: ctx.deps, replyToId: ctx.replyToId, threadId: ctx.threadId, @@ -554,6 +578,7 @@ const slackChannelOutbound: ChannelOutboundAdapter = { const { slackOutbound } = await loadSlackOutboundAdapterModule(); return await slackOutbound.sendMedia!({ ...ctx, + to, replyToId: threadTsValue, threadId: null, deliveryQueueId: undefined, @@ -595,15 +620,20 @@ const slackMessageAdapter = { ...slackMessageAdapterBase.durableFinal?.capabilities, reconcileUnknownSend: true, }, - admitDeferredDelivery: ({ cfg, accountId }) => { + admitDeferredDelivery: ({ cfg, accountId, to }) => { const effectiveAccountId = normalizeOptionalString(accountId) ?? resolveDefaultSlackAccountId(cfg); - return mergeSlackAccountConfig(cfg, effectiveAccountId).enterpriseOrgInstall === true - ? { - status: "permanent_rejection" as const, - reason: "unsupported_enterprise_slack_delivery", - } - : { status: "allowed" as const }; + const account = resolveSlackAccount({ cfg, accountId: effectiveAccountId }); + try { + const target = parseSlackTarget(to, { defaultKind: "channel" }); + assertSlackDirectSendAllowed(account, target?.teamId); + return { status: "allowed" as const }; + } catch (error) { + return { + status: "permanent_rejection" as const, + reason: error instanceof Error ? error.message : String(error), + }; + } }, reconcileUnknownSendKinds: { text: true }, reconcileUnknownSend: async (ctx) => diff --git a/extensions/slack/src/client-options.ts b/extensions/slack/src/client-options.ts index cb1a2e914c19..b39fd89b8f62 100644 --- a/extensions/slack/src/client-options.ts +++ b/extensions/slack/src/client-options.ts @@ -20,7 +20,10 @@ const requireFromSlackSocketMode = (() => { function loadSlackUndiciRuntime(): SlackUndiciRuntime { return requireFromSlackSocketMode("undici") as SlackUndiciRuntime; } -export type SlackLookupClientOptions = Pick; +export type SlackLookupClientOptions = Pick< + WebClientOptions, + "fetch" | "slackApiUrl" | "teamId" | "timeout" +>; export const SLACK_DEFAULT_RETRY_OPTIONS: RetryOptions = { retries: 2, diff --git a/extensions/slack/src/client.test.ts b/extensions/slack/src/client.test.ts index bdd8db9a2961..33cf0c646bd5 100644 --- a/extensions/slack/src/client.test.ts +++ b/extensions/slack/src/client.test.ts @@ -341,12 +341,35 @@ describe("slack web client config", () => { expect(WebClient).toHaveBeenCalledTimes(2); }); - it("only exposes API-root options on cached write clients", () => { + it("limits cached write-client options to routing scopes", () => { expectTypeOf[1]>>().toEqualTypeOf< - Pick + Pick >(); }); + it("keeps one org token partitioned by workspace", () => { + clearProxyEnvForTest(); + try { + const first = getSlackWriteClient("xoxb-org", { teamId: "T1" }); + const reused = getSlackWriteClient("xoxb-org", { teamId: "T1" }); + const second = getSlackWriteClient("xoxb-org", { teamId: "T2" }); + + expect(reused).toBe(first); + expect(second).not.toBe(first); + expect(WebClient).toHaveBeenCalledTimes(2); + expect(WebClient).toHaveBeenNthCalledWith(1, "xoxb-org", { + retryConfig: SLACK_WRITE_RETRY_OPTIONS, + teamId: "T1", + }); + expect(WebClient).toHaveBeenNthCalledWith(2, "xoxb-org", { + retryConfig: SLACK_WRITE_RETRY_OPTIONS, + teamId: "T2", + }); + } finally { + restoreProxyEnvForTest(); + } + }); + it("keeps write clients separated by Slack API URL client options", () => { clearProxyEnvForTest(); try { diff --git a/extensions/slack/src/client.ts b/extensions/slack/src/client.ts index e6da6913910c..7301f6739e6a 100644 --- a/extensions/slack/src/client.ts +++ b/extensions/slack/src/client.ts @@ -20,7 +20,7 @@ let slackListenerUploadCompletionClientCache = new WeakMap< { teamId: string; client: WebClient } >(); -type SlackWriteClientCacheOptions = Pick; +type SlackWriteClientCacheOptions = Pick; type SlackFetch = NonNullable; export { @@ -92,7 +92,9 @@ export function createSlackTokenCacheKey(token: string): string { function slackWriteClientCacheKey(token: string, options: SlackWriteClientCacheOptions): string { const tokenKey = createSlackTokenCacheKey(token); - return options.slackApiUrl ? `${tokenKey}:api:${options.slackApiUrl}` : tokenKey; + const apiScope = options.slackApiUrl ? `:api:${options.slackApiUrl}` : ""; + const teamScope = options.teamId ? `:team:${options.teamId.trim().toLowerCase()}` : ""; + return `${tokenKey}${apiScope}${teamScope}`; } export function getSlackWriteClient( diff --git a/extensions/slack/src/direct-send-admission.ts b/extensions/slack/src/direct-send-admission.ts index 64c59ba01284..1a7581219285 100644 --- a/extensions/slack/src/direct-send-admission.ts +++ b/extensions/slack/src/direct-send-admission.ts @@ -1,8 +1,12 @@ // Slack plugin module owns admission for exported/direct delivery paths. import type { ResolvedSlackAccount } from "./accounts.js"; -export function assertSlackDirectSendAllowed(account: ResolvedSlackAccount): void { - if (account.config.enterpriseOrgInstall === true) { +export function assertSlackDirectSendAllowed(account: ResolvedSlackAccount, teamId?: string): void { + const hasTeamScope = Boolean(teamId?.trim()); + if (account.config.enterpriseOrgInstall === true && !hasTeamScope) { throw new Error("unsupported_enterprise_slack_delivery"); } + if (account.config.enterpriseOrgInstall !== true && hasTeamScope) { + throw new Error("unexpected_enterprise_slack_workspace"); + } } diff --git a/extensions/slack/src/draft-stream.test.ts b/extensions/slack/src/draft-stream.test.ts index 39c661c1ee30..036e9ee8d428 100644 --- a/extensions/slack/src/draft-stream.test.ts +++ b/extensions/slack/src/draft-stream.test.ts @@ -83,9 +83,6 @@ describe("createSlackDraftStream", () => { it("uses the enterprise event client for draft writes", async () => { const client = {} as NonNullable["client"]; const eventScope = { - apiAppId: "A_TEST", - enterpriseId: "E_TEST", - isEnterpriseInstall: true as const, teamId: "T_TEST", client, }; @@ -100,7 +97,7 @@ describe("createSlackDraftStream", () => { expect(send).toHaveBeenCalledWith( "channel:C123", "hello", - expect.objectContaining({ client, enterpriseEventScope: eventScope }), + expect.objectContaining({ eventScope }), ); expect(edit).toHaveBeenCalledWith( "C123", @@ -353,9 +350,6 @@ describe("createSlackDraftStream", () => { it("keeps simultaneous Enterprise Grid conversations isolated by workspace", async () => { const accountId = "enterprise-grid"; const eventScope = { - apiAppId: "A_TEST", - enterpriseId: "E_TEST", - isEnterpriseInstall: true as const, teamId: "T_FIRST", client: {} as NonNullable["client"], }; diff --git a/extensions/slack/src/draft-stream.ts b/extensions/slack/src/draft-stream.ts index 6e46fbebf424..f8ef54abe314 100644 --- a/extensions/slack/src/draft-stream.ts +++ b/extensions/slack/src/draft-stream.ts @@ -117,9 +117,7 @@ export function createSlackDraftStream(params: { accountId: params.accountId, threadTs, identity: params.identity, - ...(params.eventScope - ? { client: params.eventScope.client, enterpriseEventScope: params.eventScope } - : {}), + ...(params.eventScope ? { eventScope: params.eventScope } : {}), ...(params.metadata ? { metadata: params.metadata } : {}), ...(blocks ? { blocks } : {}), }); diff --git a/extensions/slack/src/message-tools.test.ts b/extensions/slack/src/message-tools.test.ts index 7fc3de14bf19..1e64d7f4afdc 100644 --- a/extensions/slack/src/message-tools.test.ts +++ b/extensions/slack/src/message-tools.test.ts @@ -61,6 +61,61 @@ describe("Slack message tools", () => { ); }); + it("preserves a workspace-qualified channel for reactions", async () => { + const invoke = vi.fn(async () => ({ content: [], details: { ok: true } })); + const actions = createSlackActions("slack", { invoke }); + if (!actions.handleAction) { + throw new Error("Slack message actions must provide an executor."); + } + + await actions.handleAction({ + channel: "slack", + action: "react", + cfg: {} as OpenClawConfig, + params: { + channelId: "team:T123:channel:C123", + messageId: "123.456", + emoji: "✅", + }, + }); + + expect(invoke).toHaveBeenCalledWith( + expect.objectContaining({ + action: "react", + channelId: "team:T123:channel:C123", + }), + expect.any(Object), + undefined, + ); + }); + + it("preserves a workspace-qualified channel for implicit upload destinations", async () => { + const invoke = vi.fn(async () => ({ content: [], details: { ok: true } })); + const actions = createSlackActions("slack", { invoke }); + if (!actions.handleAction) { + throw new Error("Slack message actions must provide an executor."); + } + + await actions.handleAction({ + channel: "slack", + action: "upload-file", + cfg: {} as OpenClawConfig, + params: { + channelId: "team:T123:channel:C123", + filePath: "/tmp/report.png", + }, + }); + + expect(invoke).toHaveBeenCalledWith( + expect.objectContaining({ + action: "uploadFile", + to: "team:T123:channel:C123", + }), + expect.any(Object), + undefined, + ); + }); + it("classifies provider-native mutation actions", () => { const actions = createSlackActions("slack"); for (const action of ["sendMessage", "editMessage", "deleteMessage", "pinMessage"]) { diff --git a/extensions/slack/src/monitor/context.test.ts b/extensions/slack/src/monitor/context.test.ts index 2e134946fd51..5f43d85d5a8a 100644 --- a/extensions/slack/src/monitor/context.test.ts +++ b/extensions/slack/src/monitor/context.test.ts @@ -172,10 +172,7 @@ describe("createSlackMonitorContext channel metadata cache", () => { it("isolates remembered types by enterprise team scope", async () => { const createScope = (teamId: string): SlackEventScope => ({ - apiAppId: "A_EXPECTED", - enterpriseId: "E_EXPECTED", teamId, - isEnterpriseInstall: true, client: { conversations: { info: vi.fn().mockRejectedValue(new Error("missing_scope")) }, }, diff --git a/extensions/slack/src/monitor/event-scope.test.ts b/extensions/slack/src/monitor/event-scope.test.ts index bf0479af35d6..2818eb0aef83 100644 --- a/extensions/slack/src/monitor/event-scope.test.ts +++ b/extensions/slack/src/monitor/event-scope.test.ts @@ -17,10 +17,7 @@ describe("resolveSlackEventScope", () => { expect(result).toMatchObject({ ok: true, scope: { - apiAppId: "A123", - enterpriseId: "E123", teamId, - isEnterpriseInstall: true, client: listenerClient, }, }); @@ -39,10 +36,7 @@ describe("resolveSlackEventScope", () => { expect(result).toMatchObject({ ok: true, scope: { - apiAppId: "A123", - enterpriseId: "E123", teamId: "T111", - isEnterpriseInstall: true, client, }, }); diff --git a/extensions/slack/src/monitor/event-scope.ts b/extensions/slack/src/monitor/event-scope.ts index 91601270bcb1..1a013ec6cc11 100644 --- a/extensions/slack/src/monitor/event-scope.ts +++ b/extensions/slack/src/monitor/event-scope.ts @@ -4,17 +4,14 @@ import { normalizeOptionalString } from "openclaw/plugin-sdk/string-coerce-runti import { getSlackListenerUploadCompletionClient } from "../client.js"; import type { SlackInstallationIdentity } from "./enterprise-install.js"; -export type SlackEventScope = { - apiAppId: string; - enterpriseId: string; +export type SlackEventScope = Readonly<{ teamId: string; - isEnterpriseInstall: true; // Keep Bolt's exact listener client for ordinary reads and writes. client: WebClient; // Completion is one-shot, so uploads finalize through a team-scoped client // that cannot inherit Bolt's normal request retries. uploadCompletionClient?: WebClient; -}; +}>; type SlackEventScopeResolution = | { ok: true; scope?: SlackEventScope } @@ -86,10 +83,7 @@ export function resolveSlackEventScope(params: { return { ok: true, scope: { - apiAppId, - enterpriseId, teamId, - isEnterpriseInstall: true, client: params.client, ...(uploadCompletionClient ? { uploadCompletionClient } : {}), }, diff --git a/extensions/slack/src/monitor/message-handler/dispatch-setup.ts b/extensions/slack/src/monitor/message-handler/dispatch-setup.ts index 7142d94e964a..b35f1e22d904 100644 --- a/extensions/slack/src/monitor/message-handler/dispatch-setup.ts +++ b/extensions/slack/src/monitor/message-handler/dispatch-setup.ts @@ -83,7 +83,7 @@ export async function createSlackDispatchSetup(prepared: PreparedSlackMessage) { sessionKey: inboundLastRouteSessionKey, deliveryContext: { channel: "slack", - to: `user:${message.user}`, + to: prepared.ctxPayload.OriginatingTo ?? prepared.ctxPayload.To ?? `user:${message.user}`, accountId: route.accountId, threadId: prepared.ctxPayload.MessageThreadId ?? prepared.ctxPayload.TransportThreadId, }, diff --git a/extensions/slack/src/monitor/message-handler/dispatch.preview-fallback.test.ts b/extensions/slack/src/monitor/message-handler/dispatch.preview-fallback.test.ts index 0e0c59300dab..1df762e8bc53 100644 --- a/extensions/slack/src/monitor/message-handler/dispatch.preview-fallback.test.ts +++ b/extensions/slack/src/monitor/message-handler/dispatch.preview-fallback.test.ts @@ -321,9 +321,6 @@ function createPreparedSlackMessage(params?: { relayIdentity?: { username?: string; iconUrl?: string; iconEmoji?: string }; turnAdoptionLifecycle?: object; eventScope?: { - apiAppId: string; - enterpriseId: string; - isEnterpriseInstall: true; teamId: string; client: Record; }; @@ -405,9 +402,6 @@ async function dispatchNativeProgressScenario(params: { }; replyToMode?: "off" | "first" | "all" | "batched"; eventScope?: { - apiAppId: string; - enterpriseId: string; - isEnterpriseInstall: true; teamId: string; client: Record; }; @@ -1458,6 +1452,29 @@ describe("dispatchPreparedSlackMessage preview fallback", () => { }); }); + it("preserves a workspace-qualified DM route during dispatch", async () => { + await dispatchPreparedSlackMessage( + createPreparedSlackMessage({ + isDirectMessage: true, + message: { + channel: "D123", + user: "U1", + ts: "501.000", + }, + ctxPayload: { + OriginatingTo: "team:T123:user:U1", + SessionKey: "agent:main:main:account:default:team:t123", + }, + }), + ); + + expect(updateLastRouteMock).toHaveBeenCalledWith( + expect.objectContaining({ + deliveryContext: expect.objectContaining({ to: "team:T123:user:U1" }), + }), + ); + }); + it("uses DM transport thread metadata for last-route updates", async () => { await dispatchPreparedSlackMessage( createPreparedSlackMessage({ @@ -3144,9 +3161,6 @@ describe("dispatchPreparedSlackMessage preview fallback", () => { finalPayload: { text: FINAL_REPLY_TEXT }, events: [{ kind: "item", progressText: "checking" }], eventScope: { - apiAppId: "A_TEST", - enterpriseId: "E_TEST", - isEnterpriseInstall: true, teamId: "T_ENTERPRISE", client: eventClient, }, @@ -3992,9 +4006,6 @@ describe("dispatchPreparedSlackMessage preview fallback", () => { chat: { postMessage: postMessageMock, update: chatUpdateMock }, }; const eventScope = { - apiAppId: "A_TEST", - enterpriseId: "E_TEST", - isEnterpriseInstall: true as const, teamId: "T_ENTERPRISE", client: eventClient, }; @@ -4263,9 +4274,6 @@ describe("dispatchPreparedSlackMessage preview fallback", () => { await dispatchPreparedSlackMessage( createPreparedSlackMessage({ eventScope: { - apiAppId: "A_TEST", - enterpriseId: "E_TEST", - isEnterpriseInstall: true, teamId: "T_ENTERPRISE", client: eventClient, }, @@ -4286,9 +4294,6 @@ describe("dispatchPreparedSlackMessage preview fallback", () => { users: { info: usersInfo }, }; const eventScope = { - apiAppId: "A_TEST", - enterpriseId: "E_TEST", - isEnterpriseInstall: true as const, teamId: "T_ENTERPRISE", client: eventClient, }; diff --git a/extensions/slack/src/monitor/message-handler/dispatch.ts b/extensions/slack/src/monitor/message-handler/dispatch.ts index 197e9b78a3a4..cdee1efd5e62 100644 --- a/extensions/slack/src/monitor/message-handler/dispatch.ts +++ b/extensions/slack/src/monitor/message-handler/dispatch.ts @@ -586,7 +586,7 @@ export async function dispatchPreparedSlackMessage(prepared: PreparedSlackMessag if (anyReplyDelivered && participationThreadTs) { recordSlackThreadParticipation(account.accountId, message.channel, participationThreadTs, { agentId: route.agentId, - ...(prepared.eventScope ? { teamId: prepared.eventScope.teamId } : {}), + teamId: prepared.eventScope?.teamId, }); } if (dispatchError) { diff --git a/extensions/slack/src/monitor/message-handler/prepare.test.ts b/extensions/slack/src/monitor/message-handler/prepare.test.ts index 3a4ecc7d6f37..8d0e26fde1a1 100644 --- a/extensions/slack/src/monitor/message-handler/prepare.test.ts +++ b/extensions/slack/src/monitor/message-handler/prepare.test.ts @@ -445,14 +445,11 @@ describe("slack prepareSlackMessage inbound contract", () => { expect(prepared.ctxPayload.From).toBe("slack:U123"); }); - it("uses the validated event workspace as the standardized conversation space", async () => { + it("carries the validated event workspace through reusable DM routing", async () => { const ctx = createDefaultSlackCtx(); ctx.teamId = ""; const eventScope = { - apiAppId: "A1", - enterpriseId: "E1", - isEnterpriseInstall: true, - teamId: "T_ENTERPRISE", + teamId: "T123ENTERPRISE", client: {} as SlackEventScope["client"], } satisfies SlackEventScope; @@ -464,7 +461,53 @@ describe("slack prepareSlackMessage inbound contract", () => { }); assertPrepared(prepared, "org-wide Slack DM"); - expect(prepared.ctxPayload.GroupSpace).toBe("T_ENTERPRISE"); + expect(prepared.ctxPayload.GroupSpace).toBe("T123ENTERPRISE"); + expect(prepared.ctxPayload.To).toBe("team:T123ENTERPRISE:user:U123"); + expect(prepared.ctxPayload.OriginatingTo).toBe("team:T123ENTERPRISE:user:U123"); + expect(prepared.ctxPayload.NativeChannelId).toBe("D999"); + expect(prepared.replyTarget).toBe("channel:D999"); + expect(prepared.turn.record).toMatchObject({ + updateLastRoute: { + channel: "slack", + to: "team:T123ENTERPRISE:user:U123", + }, + }); + }); + + it("carries the validated event workspace through reusable channel routing", async () => { + const ctx = createReplyToAllSlackCtx({ + groupPolicy: "open", + defaultRequireMention: false, + asChannel: true, + }); + const eventScope = { + teamId: "T123ENTERPRISE", + client: {} as SlackEventScope["client"], + } satisfies SlackEventScope; + + const prepared = await prepareSlackMessage({ + ctx, + account: createSlackAccount({ groupPolicy: "open" }), + message: createSlackMessage({ + channel: "C123CHANNEL", + channel_type: "channel", + user: "U123", + text: "hello", + }), + opts: { source: "message", eventScope }, + }); + + assertPrepared(prepared, "org-wide Slack channel message"); + expect(prepared.ctxPayload.To).toBe("team:T123ENTERPRISE:channel:C123CHANNEL"); + expect(prepared.ctxPayload.OriginatingTo).toBe("team:T123ENTERPRISE:channel:C123CHANNEL"); + expect(prepared.ctxPayload.NativeChannelId).toBe("C123CHANNEL"); + expect(prepared.replyTarget).toBe("channel:C123CHANNEL"); + expect(prepared.turn.record).toMatchObject({ + updateLastRoute: { + channel: "slack", + to: "team:T123ENTERPRISE:channel:C123CHANNEL", + }, + }); }); it("routes a self-threaded Agent View root before capability detection completes", async () => { @@ -1026,6 +1069,7 @@ describe("slack prepareSlackMessage inbound contract", () => { source: "app_mention" | "message"; mentionType: "explicit" | "implicit" | "regex"; bindingOwner: "none" | "plugin" | "runtime"; + enterpriseTeamId?: string; expectRootMentioned?: boolean; expectFollowUpMentioned?: boolean; }; @@ -1046,7 +1090,7 @@ describe("slack prepareSlackMessage inbound contract", () => { const expectedSessionKey = scenario.bindingOwner === "runtime" ? "agent:review:slack:channel:c0ahzfcas1k" - : `agent:main:slack:channel:${channelId.toLowerCase()}:thread:${rootTs}`; + : `agent:main:slack:channel:${scenario.enterpriseTeamId ? `team:${scenario.enterpriseTeamId.toLowerCase()}:channel:` : ""}${channelId.toLowerCase()}:thread:${rootTs}`; const { storePath } = storeFixture.makeTmpStorePath(); const channelsConfig = implicit ? { [channelId]: { enabled: true, requireMention: false } } @@ -1086,6 +1130,9 @@ describe("slack prepareSlackMessage inbound contract", () => { replyToMode, ...(channelsConfig ? { channelsConfig } : {}), }); + if (scenario.enterpriseTeamId) { + Object.assign(slackCtx, { botUserId: "" }); + } slackCtx.resolveChannelName = async () => ({ name: implicit ? "genai" : "proj-openclaw", type: "channel", @@ -1148,9 +1195,19 @@ describe("slack prepareSlackMessage inbound contract", () => { opts: { source: scenario.source, ...(scenario.source === "app_mention" ? { wasMentioned: true } : {}), + ...(scenario.enterpriseTeamId + ? { + eventScope: { + teamId: scenario.enterpriseTeamId, + client: slackCtx.app.client, + }, + } + : {}), }, }); - recordSlackThreadParticipation("default", channelId, rootTs); + recordSlackThreadParticipation("default", channelId, rootTs, { + teamId: scenario.enterpriseTeamId, + }); const followUp = await prepareSlackMessage({ ctx: slackCtx, account, @@ -1163,7 +1220,17 @@ describe("slack prepareSlackMessage inbound contract", () => { ts: "1777244714.000100", thread_ts: rootTs, } as SlackMessageEvent, - opts: { source: "message" }, + opts: { + source: "message", + ...(scenario.enterpriseTeamId + ? { + eventScope: { + teamId: scenario.enterpriseTeamId, + client: slackCtx.app.client, + }, + } + : {}), + }, }); const expectedAgentId = scenario.bindingOwner === "runtime" @@ -3630,6 +3697,14 @@ Second paragraph should still reach the agent after Slack's preview cutoff.`; bindingOwner: "none", expectFollowUpMentioned: true, }, + { + name: "keeps a Grid root app mention and unmentioned thread follow-up on one parent session", + source: "app_mention", + mentionType: "explicit", + bindingOwner: "none", + enterpriseTeamId: "T123ENTERPRISE", + expectFollowUpMentioned: true, + }, { name: "keeps a message-first root mention and URL-only Slack thread follow-up on one parent session", source: "message", diff --git a/extensions/slack/src/monitor/message-handler/prepare.thread-session-key.test.ts b/extensions/slack/src/monitor/message-handler/prepare.thread-session-key.test.ts index 0d72edf6986a..eac3da80cea9 100644 --- a/extensions/slack/src/monitor/message-handler/prepare.thread-session-key.test.ts +++ b/extensions/slack/src/monitor/message-handler/prepare.thread-session-key.test.ts @@ -70,9 +70,6 @@ function buildChannelMessage(overrides?: Partial): SlackMessa function buildEventScope(teamId: string): SlackEventScope { return { - apiAppId: "A1", - enterpriseId: "E1", - isEnterpriseInstall: true, teamId, client: {} as SlackEventScope["client"], }; diff --git a/extensions/slack/src/monitor/message-handler/prepare.ts b/extensions/slack/src/monitor/message-handler/prepare.ts index 6e0317245560..385f8356fefa 100644 --- a/extensions/slack/src/monitor/message-handler/prepare.ts +++ b/extensions/slack/src/monitor/message-handler/prepare.ts @@ -46,6 +46,7 @@ import { formatSlackError } from "../../errors.js"; import { formatSlackFileReference } from "../../file-reference.js"; import type { SlackSendIdentity } from "../../send.js"; import { hasSlackThreadParticipationWithPersistence } from "../../sent-thread-cache.js"; +import { formatSlackTarget } from "../../target-parsing.js"; import type { SlackAttachment, SlackFile, SlackMessageEvent } from "../../types.js"; import { normalizeAllowListLower, normalizeSlackAllowOwnerEntry } from "../allow-list.js"; import { @@ -975,10 +976,10 @@ export async function prepareSlackMessage(params: { ) : Promise.resolve({ ok: true, name: undefined }); let implicitMentionKinds: ReturnType = []; - if (!isDirectMessage && ctx.botUserId && message.thread_ts && !wasMentioned) { + if (!isDirectMessage && message.thread_ts && !wasMentioned) { const replyToBotKinds = implicitMentionKindWhen( "reply_to_bot", - message.parent_user_id === ctx.botUserId, + Boolean(ctx.botUserId && message.parent_user_id === ctx.botUserId), ); implicitMentionKinds = replyToBotKinds.length > 0 @@ -989,7 +990,7 @@ export async function prepareSlackMessage(params: { accountId: account.accountId, channelId: message.channel, threadTs: message.thread_ts, - ...(opts.eventScope ? { teamId: opts.eventScope.teamId } : {}), + teamId: opts.eventScope?.teamId, }), ); } @@ -1182,6 +1183,22 @@ export async function prepareSlackMessage(params: { channel: "slack", accountId: account.accountId, }); + const preflightChannelTarget = opts.eventScope + ? formatSlackTarget({ + teamId: opts.eventScope.teamId, + kind: "channel", + id: message.channel, + }) + : `channel:${message.channel}`; + const replyRouteTarget = opts.eventScope + ? formatSlackTarget({ + teamId: opts.eventScope.teamId, + kind: isDirectMessage ? "user" : "channel", + id: isDirectMessage ? senderId : message.channel, + }) + : isDirectMessage + ? `user:${message.user}` + : `channel:${message.channel}`; const commandAuthorized = messageIngress.commandAccess.authorized; if (isRoomish && messageIngress.commandAccess.shouldBlockControlCommand) { @@ -1246,7 +1263,7 @@ export async function prepareSlackMessage(params: { media: preflightMedia, cfg, accountId: account.accountId, - originatingTo: `channel:${message.channel}`, + originatingTo: preflightChannelTarget, sessionKey: preflightRouting.sessionKey, messageThreadId: preflightRouting.threadContext.messageThreadId, }) @@ -1568,8 +1585,6 @@ export async function prepareSlackMessage(params: { }); } - const slackTo = isDirectMessage ? `user:${message.user}` : `channel:${message.channel}`; - const { channelMetadata, groupSystemPrompt } = resolveSlackRoomContextHints({ isRoomish, channelInfo, @@ -1652,7 +1667,7 @@ export async function prepareSlackMessage(params: { parentSessionKey: threadKeys.parentSessionKey, }, reply: { - to: slackTo, + to: replyRouteTarget, replyToId: threadContext.replyToId, messageThreadId: directThreadRoutedToDmSession ? undefined : effectiveMessageThreadId, nativeChannelId: message.channel, @@ -1762,7 +1777,7 @@ export async function prepareSlackMessage(params: { // received on. This avoids depending on a follow-up conversations.open // round-trip for the normal reply path while keeping persisted routing // metadata user-scoped for later session deliveries. - const replyTarget = isDirectMessage ? `channel:${message.channel}` : (ctxPayload.To ?? undefined); + const replyTarget = `channel:${message.channel}`; if (!replyTarget) { return null; } @@ -1772,7 +1787,7 @@ export async function prepareSlackMessage(params: { transcript: preflightAudioTranscript, cfg, accountId: account.accountId, - originatingTo: `channel:${message.channel}`, + originatingTo: preflightChannelTarget, messageThreadId: threadContext.messageThreadId, }); } @@ -1798,35 +1813,37 @@ export async function prepareSlackMessage(params: { turn: { storePath, record: { - updateLastRoute: isDirectMessage - ? { - sessionKey: updateLastRouteSessionKey, - channel: "slack", - to: `user:${message.user}`, - accountId: route.accountId, - threadId: effectiveMessageThreadId, - mainDmOwnerPin: - updateLastRouteSessionKey === route.mainSessionKey && - pinnedMainDmOwner && - message.user - ? { - ownerRecipient: pinnedMainDmOwner, - senderRecipient: normalizeLowercaseStringOrEmpty(message.user), - onSkip: ({ - ownerRecipient, - senderRecipient, - }: { - ownerRecipient: string; - senderRecipient: string; - }) => { - logVerbose( - `slack: skip main-session last route for ${senderRecipient} (pinned owner ${ownerRecipient})`, - ); - }, - } - : undefined, - } - : undefined, + updateLastRoute: + isDirectMessage || opts.eventScope + ? { + sessionKey: updateLastRouteSessionKey, + channel: "slack", + to: replyRouteTarget, + accountId: route.accountId, + threadId: effectiveMessageThreadId, + mainDmOwnerPin: + isDirectMessage && + updateLastRouteSessionKey === route.mainSessionKey && + pinnedMainDmOwner && + message.user + ? { + ownerRecipient: pinnedMainDmOwner, + senderRecipient: normalizeLowercaseStringOrEmpty(message.user), + onSkip: ({ + ownerRecipient, + senderRecipient, + }: { + ownerRecipient: string; + senderRecipient: string; + }) => { + logVerbose( + `slack: skip main-session last route for ${senderRecipient} (pinned owner ${ownerRecipient})`, + ); + }, + } + : undefined, + } + : undefined, onRecordError: (err: unknown) => { ctx.logger.warn( { diff --git a/extensions/slack/src/monitor/replies.test.ts b/extensions/slack/src/monitor/replies.test.ts index c9f17b0d920f..fb4e027c56ed 100644 --- a/extensions/slack/src/monitor/replies.test.ts +++ b/extensions/slack/src/monitor/replies.test.ts @@ -185,9 +185,6 @@ describe("deliverReplies identity passthrough", () => { const metadata = { event_type: "openclaw_test", event_payload: { source: "chart" } }; const listenerClient = { chat: { postMessage: vi.fn() } } as never; const eventScope = { - apiAppId: "A1", - enterpriseId: "E1", - isEnterpriseInstall: true as const, teamId: "T1", client: listenerClient, }; @@ -231,8 +228,7 @@ describe("deliverReplies identity passthrough", () => { mediaUrl: "https://example.com/report.png", threadTs: "thread-ts", accountId: "work", - client: listenerClient, - enterpriseEventScope: eventScope, + eventScope, textLimit: 4000, mediaMaxBytes: 1024, identity, @@ -243,8 +239,7 @@ describe("deliverReplies identity passthrough", () => { token: "xoxb-test", threadTs: "thread-ts", accountId: "work", - client: listenerClient, - enterpriseEventScope: eventScope, + eventScope, textLimit: 4000, mediaMaxBytes: 1024, blocks: [ @@ -284,13 +279,10 @@ describe("deliverReplies identity passthrough", () => { expect(options).not.toHaveProperty("identity"); }); - it("forwards the validated Enterprise event scope and exact listener client", async () => { + it("forwards the validated Enterprise event scope", async () => { sendMock.mockResolvedValue({ messageId: "123.456", channelId: "C123" }); const listenerClient = { chat: { postMessage: vi.fn() } } as never; const eventScope = { - apiAppId: "A1", - enterpriseId: "E1", - isEnterpriseInstall: true as const, teamId: "T1", client: listenerClient, }; @@ -304,8 +296,7 @@ describe("deliverReplies identity passthrough", () => { ); const options = requireSendCall()[2]; - expect(options.client).toBe(listenerClient); - expect(options.enterpriseEventScope).toBe(eventScope); + expect(options.eventScope).toBe(eventScope); expect(options.textLimit).toBe(4000); expect(options.mediaMaxBytes).toBe(1024); }); diff --git a/extensions/slack/src/monitor/replies.ts b/extensions/slack/src/monitor/replies.ts index 1695ba4c0e5f..66e316bf1360 100644 --- a/extensions/slack/src/monitor/replies.ts +++ b/extensions/slack/src/monitor/replies.ts @@ -173,8 +173,7 @@ export async function deliverReplies(params: { ...(input.textIsSlackPlainText ? { textIsSlackPlainText: true } : {}), ...(params.eventScope ? { - client: params.eventScope.client, - enterpriseEventScope: params.eventScope, + eventScope: params.eventScope, textLimit: params.textLimit, ...(params.mediaMaxBytes !== undefined ? { mediaMaxBytes: params.mediaMaxBytes } : {}), } diff --git a/extensions/slack/src/send.enterprise.test.ts b/extensions/slack/src/send.enterprise.test.ts index ef0ac33ab89b..6f5c1e50e3c6 100644 --- a/extensions/slack/src/send.enterprise.test.ts +++ b/extensions/slack/src/send.enterprise.test.ts @@ -15,6 +15,7 @@ const loadOutboundMediaFromUrl = vi.hoisted(() => })), ); const fetchWithSsrFGuard = vi.hoisted(() => vi.fn()); +const getSlackWriteClientMock = vi.hoisted(() => vi.fn()); vi.mock("openclaw/plugin-sdk/fetch-runtime", () => ({ withTrustedEnvProxyGuardedFetchMode: (value: unknown) => value, @@ -24,6 +25,10 @@ vi.mock("./runtime-api.js", async () => { const actual = await vi.importActual("./runtime-api.js"); return { ...actual, loadOutboundMediaFromUrl }; }); +vi.mock("./client.js", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, getSlackWriteClient: getSlackWriteClientMock }; +}); const { sendMessageSlack } = await import("./send.js"); @@ -63,15 +68,8 @@ function createEnterpriseClient(): EnterpriseTestClient { } as unknown as EnterpriseTestClient; } -function enterpriseEventScope( - client: WebClient, - teamId = "T1", - uploadCompletionClient: WebClient = client, -) { +function eventScope(client: WebClient, teamId = "T1", uploadCompletionClient: WebClient = client) { return { - apiAppId: "A1", - enterpriseId: "E1", - isEnterpriseInstall: true as const, teamId, client, uploadCompletionClient, @@ -85,8 +83,7 @@ function enterpriseOptions( ) { return { cfg: ENTERPRISE_CFG, - client, - enterpriseEventScope: enterpriseEventScope(client, teamId, uploadCompletionClient), + eventScope: eventScope(client, teamId, uploadCompletionClient), }; } @@ -111,6 +108,7 @@ describe("sendMessageSlack Enterprise listener scope", () => { clearSlackThreadParticipationCache(); loadOutboundMediaFromUrl.mockClear(); fetchWithSsrFGuard.mockReset(); + getSlackWriteClientMock.mockReset(); }); afterEach(() => { @@ -132,16 +130,28 @@ describe("sendMessageSlack Enterprise listener scope", () => { expect(client.chat.postMessage).not.toHaveBeenCalled(); }); - it("requires the exact validated listener client and an Enterprise account", async () => { - const client = createEnterpriseClient(); - const otherClient = createEnterpriseClient(); + it("creates a workspace-scoped client for a qualified detached send", async () => { + const scopedClient = createEnterpriseClient(); + const injectedClient = createEnterpriseClient(); + getSlackWriteClientMock.mockReturnValue(scopedClient); - await expect( - sendMessageSlack("channel:C123", "hello", { - ...enterpriseOptions(client), - client: otherClient, - }), - ).rejects.toThrow("invalid_enterprise_slack_listener_scope"); + await sendMessageSlack("team:T123:channel:C08GQH53EJM", "hello", { + cfg: ENTERPRISE_CFG, + token: "xoxb-enterprise", + client: injectedClient, + }); + + expect(getSlackWriteClientMock).toHaveBeenCalledWith("xoxb-enterprise", { + teamId: "T123", + }); + expect(scopedClient.chat.postMessage).toHaveBeenCalledWith( + expect.objectContaining({ channel: "C08GQH53EJM", text: "hello" }), + ); + expect(injectedClient.chat.postMessage).not.toHaveBeenCalled(); + }); + + it("requires an Enterprise account for an event scope", async () => { + const client = createEnterpriseClient(); await expect( sendMessageSlack("channel:C123", "hello", { ...enterpriseOptions(client), @@ -149,7 +159,6 @@ describe("sendMessageSlack Enterprise listener scope", () => { }), ).rejects.toThrow("unexpected_enterprise_slack_listener_scope"); expect(client.chat.postMessage).not.toHaveBeenCalled(); - expect(otherClient.chat.postMessage).not.toHaveBeenCalled(); }); it("uses the exact listener client without a token or team_id method payload", async () => { @@ -217,11 +226,10 @@ describe("sendMessageSlack Enterprise listener scope", () => { const replacementClient = createEnterpriseClient(); const firstDeferred = deferredPostMessage("1.000"); firstClient.chat.postMessage.mockReturnValueOnce(firstDeferred.promise); - const secondScope = enterpriseEventScope(secondClient, "T1"); + const secondScope = eventScope(secondClient, "T1"); const secondOptions = { cfg: ENTERPRISE_CFG, - client: secondClient as WebClient, - enterpriseEventScope: secondScope, + eventScope: secondScope, }; const first = sendMessageSlack("C123", "first", enterpriseOptions(firstClient, "T1")); @@ -230,7 +238,6 @@ describe("sendMessageSlack Enterprise listener scope", () => { await Promise.resolve(); expect(secondClient.chat.postMessage).not.toHaveBeenCalled(); - secondOptions.client = replacementClient; secondScope.client = replacementClient; firstDeferred.release(); await Promise.all([first, second]); @@ -286,14 +293,13 @@ describe("sendMessageSlack Enterprise listener scope", () => { it("rejects Enterprise media before upload without the one-shot completion client", async () => { const client = createEnterpriseClient(); - const scope = enterpriseEventScope(client); + const scope = eventScope(client); delete (scope as { uploadCompletionClient?: WebClient }).uploadCompletionClient; await expect( sendMessageSlack("C123", "caption", { cfg: ENTERPRISE_CFG, - client, - enterpriseEventScope: scope, + eventScope: scope, mediaUrl: "https://example.com/image.png", }), ).rejects.toThrow("missing_enterprise_slack_upload_completion_client"); diff --git a/extensions/slack/src/send.identity-fallback.test.ts b/extensions/slack/src/send.identity-fallback.test.ts index a0f529d4982a..0e8d8e9afda2 100644 --- a/extensions/slack/src/send.identity-fallback.test.ts +++ b/extensions/slack/src/send.identity-fallback.test.ts @@ -104,10 +104,6 @@ describe("sendMessageSlack customize-scope fallback", () => { { 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" }); diff --git a/extensions/slack/src/send.reconcile.test.ts b/extensions/slack/src/send.reconcile.test.ts index 3b95c75bfb31..f48a082336da 100644 --- a/extensions/slack/src/send.reconcile.test.ts +++ b/extensions/slack/src/send.reconcile.test.ts @@ -82,6 +82,15 @@ function createUnknownSendContext( }; } +function reconcileWithClient( + ctx: ChannelMessageUnknownSendContext, + client: SlackReconcileTestClient, +) { + slackClientMocks.createSlackReadClient.mockReturnValue(client); + slackClientMocks.getSlackWriteClient.mockReturnValue(client); + return reconcileSlackUnknownSend(ctx); +} + async function postWithDeliveryMetadata(params: { client: SlackReconcileTestClient; queueId?: string; @@ -109,6 +118,37 @@ describe("reconcileSlackUnknownSend", () => { slackClientMocks.getSlackWriteClient.mockReset(); }); + it("uses workspace-scoped clients for an Enterprise reconciliation", async () => { + const readClient = createSlackReconcileTestClient(); + const writeClient = createSlackReconcileTestClient(); + slackClientMocks.createSlackReadClient.mockReturnValue(readClient); + slackClientMocks.getSlackWriteClient.mockReturnValue(writeClient); + + await reconcileSlackUnknownSend( + createUnknownSendContext({ + cfg: { + channels: { + slack: { + botToken: "xoxb-org", + enterpriseOrgInstall: true, + }, + }, + } as OpenClawConfig, + to: "team:T123:channel:C123", + }), + ); + + expect(slackClientMocks.createSlackReadClient).toHaveBeenCalledWith("xoxb-org", { + teamId: "T123", + }); + expect(slackClientMocks.getSlackWriteClient).toHaveBeenCalledWith("xoxb-org", { + teamId: "T123", + }); + expect(readClient.conversations.history).toHaveBeenCalledWith( + expect.objectContaining({ channel: "C123" }), + ); + }); + it("attaches an opaque durable id and reconciles the exact posted message", async () => { const client = createSlackReconcileTestClient(); const metadata = await postWithDeliveryMetadata({ client }); @@ -120,7 +160,7 @@ describe("reconcileSlackUnknownSend", () => { ], }); - const result = await reconcileSlackUnknownSend(createUnknownSendContext(), { client }); + const result = await reconcileWithClient(createUnknownSendContext(), client); expect(client.conversations.history).toHaveBeenCalledWith({ channel: "C123", @@ -156,7 +196,7 @@ describe("reconcileSlackUnknownSend", () => { messages: [{ ts: "1782584647.000002", metadata }], }); - const reconciled = await reconcileSlackUnknownSend(createUnknownSendContext(), { client }); + const reconciled = await reconcileWithClient(createUnknownSendContext(), client); expect(reconciled.status).toBe("sent"); if (reconciled.status === "sent") { expect(reconciled.receipt.platformMessageIds).toEqual(["1782584647.000002"]); @@ -261,7 +301,7 @@ describe("reconcileSlackUnknownSend", () => { ], }); - const reconciled = await reconcileSlackUnknownSend(createUnknownSendContext(), { client }); + const reconciled = await reconcileWithClient(createUnknownSendContext(), client); expect(reconciled.status).toBe("sent"); if (reconciled.status === "sent") { expect(reconciled.receipt.platformMessageIds).toEqual([ @@ -375,8 +415,12 @@ describe("reconcileSlackUnknownSend", () => { await expect( reconcileSlackUnknownSend(createUnknownSendContext({ cfg: tokenCfg })), ).resolves.toEqual(expect.objectContaining({ status: "sent" })); - expect(slackClientMocks.createSlackReadClient).toHaveBeenCalledWith("xoxp-read"); - expect(slackClientMocks.getSlackWriteClient).toHaveBeenCalledWith("xoxb-write"); + expect(slackClientMocks.createSlackReadClient).toHaveBeenCalledWith("xoxp-read", { + teamId: undefined, + }); + expect(slackClientMocks.getSlackWriteClient).toHaveBeenCalledWith("xoxb-write", { + teamId: undefined, + }); expect(readClient.conversations.history).toHaveBeenCalledOnce(); expect(writeClient.conversations.history).not.toHaveBeenCalled(); }); @@ -442,15 +486,13 @@ describe("reconcileSlackUnknownSend", () => { messages: [{ ts: "1782584647.000002", text: "final answer" }], }); - await expect( - reconcileSlackUnknownSend(createUnknownSendContext(), { client }), - ).resolves.toEqual({ + await expect(reconcileWithClient(createUnknownSendContext(), client)).resolves.toEqual({ status: "unresolved", error: "Slack history contains no exact durable delivery marker", retryable: true, }); await expect( - reconcileSlackUnknownSend(createUnknownSendContext({ retryCount: 2 }), { client }), + reconcileWithClient(createUnknownSendContext({ retryCount: 2 }), client), ).resolves.toEqual({ status: "unresolved", error: "Slack history contains no exact durable delivery marker", @@ -474,13 +516,13 @@ describe("reconcileSlackUnknownSend", () => { ], }); - const result = await reconcileSlackUnknownSend( + const result = await reconcileWithClient( createUnknownSendContext({ threadId: "1782584644.111111", payloads: [{ text: "final answer", replyToId: "1782584644.222222" }], effectiveReplyToId: "1782584644.377229", }), - { client }, + client, ); expect(client.conversations.replies).toHaveBeenCalledWith( @@ -523,7 +565,7 @@ describe("reconcileSlackUnknownSend", () => { messages: [{ ts: "1782584647.000002", metadata }], }); - const result = await reconcileSlackUnknownSend(createUnknownSendContext(overrides), { client }); + const result = await reconcileWithClient(createUnknownSendContext(overrides), client); expect(result.status).toBe("sent"); expect(client.conversations.history).toHaveBeenCalledOnce(); @@ -543,9 +585,9 @@ describe("reconcileSlackUnknownSend", () => { messages: [{ ts: "1782584647.000002", metadata }], }); - await expect( - reconcileSlackUnknownSend(createUnknownSendContext(), { client }), - ).resolves.toEqual(expect.objectContaining({ status: "sent", messageId: "1782584647.000002" })); + await expect(reconcileWithClient(createUnknownSendContext(), client)).resolves.toEqual( + expect.objectContaining({ status: "sent", messageId: "1782584647.000002" }), + ); expect(client.conversations.history).toHaveBeenNthCalledWith( 2, expect.objectContaining({ cursor: "cursor-2" }), @@ -594,9 +636,9 @@ describe("reconcileSlackUnknownSend", () => { })), }); - const result = await reconcileSlackUnknownSend( + const result = await reconcileWithClient( createUnknownSendContext({ cfg: chunkedCfg, payloads: [{ text: "final answer" }] }), - { client }, + client, ); expect(result.status).toBe("sent"); if (result.status === "sent") { @@ -628,9 +670,9 @@ describe("reconcileSlackUnknownSend", () => { ], }); await expect( - reconcileSlackUnknownSend( + reconcileWithClient( createUnknownSendContext({ cfg: chunkedCfg, payloads: [{ text: "final answer" }] }), - { client }, + client, ), ).resolves.toEqual({ status: "unresolved", @@ -660,7 +702,7 @@ describe("reconcileSlackUnknownSend", () => { ], }); - const reconciled = await reconcileSlackUnknownSend(createUnknownSendContext(), { client }); + const reconciled = await reconcileWithClient(createUnknownSendContext(), client); expect(reconciled.status).toBe("sent"); if (reconciled.status === "sent") { diff --git a/extensions/slack/src/send.ts b/extensions/slack/src/send.ts index b0db56848438..376cf27c3d64 100644 --- a/extensions/slack/src/send.ts +++ b/extensions/slack/src/send.ts @@ -41,14 +41,17 @@ import { } from "./client-delivery.js"; import { createSlackReadClient, createSlackTokenCacheKey, getSlackWriteClient } from "./client.js"; import { assertSlackDirectSendAllowed } from "./direct-send-admission.js"; +import { formatSlackError } from "./errors.js"; import { chunkSlackMrkdwnText, markdownToSlackMrkdwnChunks } from "./format.js"; import { SLACK_EDIT_TEXT_MAX_BYTES, SLACK_TEXT_LIMIT } from "./limits.js"; +import type { SlackEventScope } from "./monitor/event-scope.js"; import { buildSlackNativeDataAccessibilityText, hasSlackNativeDataBlock, isSlackInvalidBlocksError, } from "./native-data-blocks.js"; import { buildSlackNativeDataDeliveryPlan } from "./native-data-fallback.js"; +import type { SlackUnfurlOptions } from "./post-message-payload.js"; import { resolveSlackQuestionActionIds, SLACK_QUESTION_FINALIZATION_BLOCKS, @@ -75,10 +78,12 @@ type SlackRecipient = | { kind: "user"; id: string; + teamId?: string; } | { kind: "channel"; id: string; + teamId?: string; }; export type SlackSendIdentity = { @@ -87,19 +92,18 @@ export type SlackSendIdentity = { iconEmoji?: string; }; -type SlackEnterpriseEventScope = Readonly<{ - apiAppId: string; - enterpriseId: string; - teamId: string; - isEnterpriseInstall: true; +type SlackResolvedDelivery = Readonly<{ client: WebClient; - uploadCompletionClient?: WebClient; -}>; - -type SlackEnterpriseDelivery = Readonly<{ - client: WebClient; - teamId: string; - uploadCompletionClient?: WebClient; + credential: string; + identity?: SlackSendIdentity; + recipient: SlackRecipient; + requireMessageTimestamp: boolean; + teamId?: string; + unfurl: SlackUnfurlOptions; + upload?: Readonly<{ + completionClient: WebClient; + auditContext: string; + }>; }>; const slackDefaultSendIdentities = new Map(); @@ -118,8 +122,8 @@ type SlackSendOpts = { mediaLocalRoots?: readonly string[]; mediaReadFile?: (filePath: string) => Promise; client?: WebClient; - /** Monitor-private proof that `client` belongs to the validated Enterprise event turn. */ - enterpriseEventScope?: SlackEnterpriseEventScope; + /** Monitor-private listener context validated from the active event. */ + eventScope?: SlackEventScope; /** Monitor-private delivery limits already resolved for the active listener. */ textLimit?: number; /** Slack-private marker for text that is already safe mrkdwn and must not be parsed again. */ @@ -365,6 +369,7 @@ function parseRecipient(raw: string): SlackRecipient { return { kind: target.kind, id: canonicalizeSlackApiTargetId(target.kind, target.id, raw), + teamId: target.teamId, }; } @@ -376,31 +381,96 @@ function parseEnterpriseEventRecipient(raw: string): SlackRecipient { return { kind: "channel", id: canonicalizeSlackApiTargetId("channel", match[1]) }; } -function resolveEnterpriseEventScope(params: { +function resolveSlackSendEventScope(params: { account: ReturnType; opts: SlackSendOpts; -}): SlackEnterpriseEventScope | undefined { - const scope = params.opts.enterpriseEventScope; +}): SlackEventScope | undefined { + const scope = params.opts.eventScope; if (!scope) { - assertSlackDirectSendAllowed(params.account); return undefined; } if (params.account.config.enterpriseOrgInstall !== true) { throw new Error("unexpected_enterprise_slack_listener_scope"); } - if ( - !scope.isEnterpriseInstall || - !normalizeOptionalString(scope.apiAppId) || - !normalizeOptionalString(scope.enterpriseId) || - !/^T[A-Z0-9]+$/i.test(scope.teamId) || - !scope.client || - params.opts.client !== scope.client - ) { + if (!/^T[A-Z0-9]+$/i.test(scope.teamId) || !scope.client) { throw new Error("invalid_enterprise_slack_listener_scope"); } return scope; } +function resolveSlackDelivery(params: { + account: ReturnType; + eventScope?: SlackEventScope; + opts: Readonly; + recipient: SlackRecipient; +}): SlackResolvedDelivery { + if (params.eventScope) { + if (params.opts.mediaUrl && !params.eventScope.uploadCompletionClient) { + throw new Error("missing_enterprise_slack_upload_completion_client"); + } + return Object.freeze({ + client: params.eventScope.client, + credential: SLACK_ENTERPRISE_LISTENER_QUEUE_CREDENTIAL, + identity: normalizeSlackSendIdentity(params.opts.identity), + recipient: params.recipient, + requireMessageTimestamp: true, + teamId: params.eventScope.teamId, + unfurl: { unfurlMedia: params.account.config.unfurlMedia }, + ...(params.eventScope.uploadCompletionClient + ? { + upload: Object.freeze({ + completionClient: params.eventScope.uploadCompletionClient, + auditContext: "slack-enterprise-immediate-upload", + }), + } + : {}), + }); + } + + const credential = resolveToken({ + explicit: params.opts.token, + accountId: params.account.accountId, + fallbackToken: resolveSlackOperationToken(params.account, "write"), + fallbackSource: + params.account.identity === "user" + ? params.account.userTokenSource + : params.account.botTokenSource, + }); + return Object.freeze({ + client: params.recipient.teamId + ? getSlackWriteClient(credential, { teamId: params.recipient.teamId }) + : (params.opts.client ?? getSlackWriteClient(credential)), + credential, + identity: resolveSlackSendIdentity({ + accountId: params.account.accountId, + explicit: params.opts.identity, + }), + recipient: params.recipient, + requireMessageTimestamp: false, + teamId: params.recipient.teamId, + unfurl: params.recipient.teamId + ? { unfurlMedia: params.account.config.unfurlMedia } + : { + unfurlLinks: params.account.config.unfurlLinks, + unfurlMedia: params.account.config.unfurlMedia, + }, + }); +} + +function assertSlackPostMessageResponse( + response: { ok?: boolean; ts?: string; error?: unknown }, + required: boolean, +): void { + if (!required || (response.ok && response.ts)) { + return; + } + throw new Error( + response.ok + ? "Slack chat.postMessage returned no message timestamp" + : `Slack chat.postMessage failed: ${formatSlackError(response.error, "unknown error")}`, + ); +} + function resolveSlackTextChunkLimit(params: { cfg: OpenClawConfig; accountId?: string; @@ -870,7 +940,6 @@ async function scanSlackConversationForDelivery(params: { export async function reconcileSlackUnknownSend( ctx: ChannelMessageUnknownSendContext, - opts?: { client?: SlackConversationLookupClient }, ): Promise { const cfg = requireRuntimeConfig(ctx.cfg, "Slack delivery reconciliation"); const account = resolveSlackAccount({ @@ -885,6 +954,16 @@ export async function reconcileSlackUnknownSend( retryable: false, }; } + const recipient = parseRecipient(ctx.to); + try { + assertSlackDirectSendAllowed(account, recipient.teamId); + } catch (error) { + return { + status: "unresolved", + error: error instanceof Error ? error.message : String(error), + retryable: false, + }; + } const readToken = resolveSlackOperationToken(account, "read"); if (!readToken) { return { @@ -893,7 +972,6 @@ export async function reconcileSlackUnknownSend( retryable: false, }; } - const recipient = parseRecipient(ctx.to); const userRecipient = isSlackUserRecipient(recipient); const writeToken = resolveSlackOperationToken(account, "write"); if (userRecipient && !writeToken) { @@ -903,8 +981,10 @@ export async function reconcileSlackUnknownSend( retryable: false, }; } - const readClient = opts?.client ?? createSlackReadClient(readToken); - const writeClient = opts?.client ?? (writeToken ? getSlackWriteClient(writeToken) : undefined); + const readClient = createSlackReadClient(readToken, { teamId: recipient.teamId }); + const writeClient = writeToken + ? getSlackWriteClient(writeToken, { teamId: recipient.teamId }) + : undefined; const payloadReplyToId = ctx.payloads[0]?.replyToId; const effectiveReplyToId = Object.hasOwn(ctx, "effectiveReplyToId") ? normalizeOptionalString(ctx.effectiveReplyToId) @@ -935,9 +1015,12 @@ export async function reconcileSlackUnknownSend( accountId: account.accountId, token: channelToken, }); - const lookupClients = opts?.client - ? [opts.client] - : [readClient, ...(writeClient && writeToken !== readToken ? [writeClient] : [])]; + const lookupClients = [ + readClient, + ...(writeClient && writeClient !== readClient && writeToken !== readToken + ? [writeClient] + : []), + ]; let lookupError: unknown; let bestUnresolvedScan: SlackConversationDeliveryScan | undefined; for (const lookupClient of lookupClients) { @@ -993,16 +1076,11 @@ export async function sendMessageSlack( cfg, accountId: opts.accountId, }); - const enterpriseEventScope = resolveEnterpriseEventScope({ account, opts }); - const enterpriseDelivery = enterpriseEventScope - ? Object.freeze({ - client: enterpriseEventScope.client, - teamId: enterpriseEventScope.teamId, - ...(enterpriseEventScope.uploadCompletionClient - ? { uploadCompletionClient: enterpriseEventScope.uploadCompletionClient } - : {}), - }) - : undefined; + const eventScope = resolveSlackSendEventScope({ account, opts }); + const recipient = eventScope ? parseEnterpriseEventRecipient(to) : parseRecipient(to); + if (!eventScope) { + assertSlackDirectSendAllowed(account, recipient.teamId); + } if (isSilentReplyText(normalizedMessage) && !opts.mediaUrl && !opts.blocks) { logVerbose("slack send: suppressed NO_REPLY token before API call"); return { @@ -1015,47 +1093,30 @@ export async function sendMessageSlack( if (!normalizedMessage && !opts.mediaUrl && !blocks) { throw new Error("Slack send requires text, blocks, or media"); } - const token = enterpriseDelivery - ? SLACK_ENTERPRISE_LISTENER_QUEUE_CREDENTIAL - : resolveToken({ - explicit: opts.token, - accountId: account.accountId, - fallbackToken: resolveSlackOperationToken(account, "write"), - fallbackSource: - account.identity === "user" ? account.userTokenSource : account.botTokenSource, - }); - const recipient = enterpriseDelivery ? parseEnterpriseEventRecipient(to) : parseRecipient(to); + const queuedOpts = Object.freeze({ ...opts }); + const delivery = resolveSlackDelivery({ account, eventScope, opts: queuedOpts, recipient }); const queueKey = createSlackSendQueueKey({ accountId: account.accountId, - token, - recipient, + token: delivery.credential, + recipient: delivery.recipient, threadTs: opts.threadTs, - ...(enterpriseDelivery ? { teamId: enterpriseDelivery.teamId } : {}), + teamId: delivery.teamId, }); - const queuedOpts = enterpriseDelivery - ? Object.freeze({ ...opts, client: enterpriseDelivery.client }) - : opts; const result = await runQueuedSlackSend(queueKey, () => sendMessageSlackQueued({ trimmedMessage, opts: queuedOpts, cfg, account, - token, - recipient, blocks, - ...(enterpriseDelivery ? { enterpriseDelivery } : {}), + delivery, }), ); const threadTs = result.threadTs ?? normalizeSlackThreadTsCandidate(queuedOpts.threadTs); if (threadTs && result.channelId && account.accountId) { - if (enterpriseDelivery) { - recordSlackThreadParticipation(account.accountId, result.channelId, threadTs, { - teamId: enterpriseDelivery.teamId, - }); - } else { - recordSlackThreadParticipation(account.accountId, result.channelId, threadTs); - } + recordSlackThreadParticipation(account.accountId, result.channelId, threadTs, { + teamId: delivery.teamId, + }); } return result; } @@ -1065,10 +1126,8 @@ async function sendMessageSlackQueued(params: { opts: SlackSendOpts; cfg: OpenClawConfig; account: ReturnType; - token: string; - recipient: SlackRecipient; blocks?: (Block | KnownBlock)[]; - enterpriseDelivery?: SlackEnterpriseDelivery; + delivery: SlackResolvedDelivery; }): Promise { try { return await sendMessageSlackQueuedInner(params); @@ -1082,29 +1141,14 @@ async function sendMessageSlackQueuedInner(params: { opts: SlackSendOpts; cfg: OpenClawConfig; account: ReturnType; - token: string; - recipient: SlackRecipient; blocks?: (Block | KnownBlock)[]; - enterpriseDelivery?: SlackEnterpriseDelivery; + delivery: SlackResolvedDelivery; }): Promise { - const { opts, cfg, account, token, recipient, blocks, trimmedMessage, enterpriseDelivery } = - params; - const client = enterpriseDelivery?.client ?? opts.client ?? getSlackWriteClient(token); - const identity = enterpriseDelivery - ? normalizeSlackSendIdentity(opts.identity) - : resolveSlackSendIdentity({ - accountId: account.accountId, - explicit: opts.identity, - }); + const { opts, cfg, account, blocks, trimmedMessage, delivery } = params; + const { client, identity, recipient, unfurl } = delivery; if (opts.replyBroadcast && opts.mediaUrl) { throw new Error("Slack replyBroadcast is only supported for text or block thread replies."); } - const unfurl = enterpriseDelivery - ? { unfurlMedia: account.config.unfurlMedia } - : { - unfurlLinks: account.config.unfurlLinks, - unfurlMedia: account.config.unfurlMedia, - }; // Durable signatures bind the concrete provider channel, so user-targeted // sends must resolve U... to the resulting D... conversation first. const directUserPostChannelId = opts.deliveryQueueId @@ -1118,7 +1162,7 @@ async function sendMessageSlackQueuedInner(params: { ? { channelId: directUserPostChannelId } : await resolveChannelId(client, recipient, { accountId: account.accountId, - token, + token: delivery.credential, }); const reportDelivery = async ( result: SlackSendResult, @@ -1239,13 +1283,7 @@ async function sendMessageSlackQueuedInner(params: { ...(usesOrderedBlockAccessibility ? { mrkdwn: false } : {}), unfurl, }); - if (enterpriseDelivery && (!response.ok || !response.ts)) { - throw new Error( - response.ok - ? "Slack chat.postMessage returned no message timestamp" - : `Slack chat.postMessage failed: ${response.error ?? "unknown error"}`, - ); - } + assertSlackPostMessageResponse(response, delivery.requireMessageTimestamp); const messageId = response.ts ?? "unknown"; deliveredChannelId = resolvePostedMessageChannelId(response, channelId); const deliveredThreadTs = @@ -1299,13 +1337,7 @@ async function sendMessageSlackQueuedInner(params: { unfurl, }); const response = posted.response; - if (enterpriseDelivery && (!response.ok || !response.ts)) { - throw new Error( - response.ok - ? "Slack chat.postMessage returned no message timestamp" - : `Slack chat.postMessage failed: ${response.error ?? "unknown error"}`, - ); - } + assertSlackPostMessageResponse(response, delivery.requireMessageTimestamp); sendIdentity = posted.identity; lastMessageId = response.ts ?? lastMessageId; deliveredChannelId = resolvePostedMessageChannelId(response, deliveredChannelId); @@ -1376,15 +1408,10 @@ async function sendMessageSlackQueuedInner(params: { let chunksToPost: string[]; if (opts.mediaUrl) { - if (enterpriseDelivery && !enterpriseDelivery.uploadCompletionClient) { - throw new Error("missing_enterprise_slack_upload_completion_client"); - } const [firstChunk, ...rest] = resolvedChunks; lastMessageId = await uploadSlackFile({ client, - ...(enterpriseDelivery?.uploadCompletionClient - ? { completionClient: enterpriseDelivery.uploadCompletionClient } - : {}), + ...(delivery.upload ? { completionClient: delivery.upload.completionClient } : {}), channelId, mediaUrl: opts.mediaUrl, mediaAccess: opts.mediaAccess, @@ -1396,7 +1423,7 @@ async function sendMessageSlackQueuedInner(params: { threadTs: opts.threadTs, maxBytes: mediaMaxBytes, onPlatformSendDispatch: dispatchOnce, - ...(enterpriseDelivery ? { auditContext: "slack-enterprise-immediate-upload" } : {}), + ...(delivery.upload ? { auditContext: delivery.upload.auditContext } : {}), }); sentMessageIds.push(lastMessageId); await reportDelivery({ @@ -1445,13 +1472,7 @@ async function sendMessageSlackQueuedInner(params: { unfurl, }); const response = posted.response; - if (enterpriseDelivery && (!response.ok || !response.ts)) { - throw new Error( - response.ok - ? "Slack chat.postMessage returned no message timestamp" - : `Slack chat.postMessage failed: ${response.error ?? "unknown error"}`, - ); - } + assertSlackPostMessageResponse(response, delivery.requireMessageTimestamp); sendIdentity = posted.identity; lastMessageId = response.ts ?? lastMessageId; deliveredChannelId = resolvePostedMessageChannelId(response, deliveredChannelId); diff --git a/extensions/slack/src/target-parsing.ts b/extensions/slack/src/target-parsing.ts index 957284f61a6f..c4958e70a4fb 100644 --- a/extensions/slack/src/target-parsing.ts +++ b/extensions/slack/src/target-parsing.ts @@ -11,7 +11,9 @@ import { export type SlackTargetKind = MessagingTargetKind; -export type SlackTarget = MessagingTarget; +export type SlackTarget = MessagingTarget & { + teamId?: string; +}; export type SlackTargetParseOptions = MessagingTargetParseOptions; @@ -19,6 +21,58 @@ export type SlackTargetParseOptions = MessagingTargetParseOptions; // 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; +const SLACK_QUALIFIED_TARGET_RE = /^team:([^:]+):(user|channel):([^:]+)$/i; + +function decodeSlackTargetPart(raw: string): string | undefined { + try { + return decodeURIComponent(raw).trim() || undefined; + } catch { + return undefined; + } +} + +function parseQualifiedSlackTarget(raw: string): SlackTarget | undefined { + const match = SLACK_QUALIFIED_TARGET_RE.exec(raw); + if (!match) { + if (/^team:/i.test(raw)) { + throw new Error( + "Slack workspace targets require team::channel: or team::user:", + ); + } + return undefined; + } + const teamId = decodeSlackTargetPart(match[1] ?? ""); + const kind = match[2]?.toLowerCase() as SlackTargetKind | undefined; + const id = decodeSlackTargetPart(match[3] ?? ""); + const idPattern = kind === "user" ? /^[UW][A-Z0-9]+$/i : /^[CDG][A-Z0-9]+$/i; + if (!teamId || !/^T[A-Z0-9]+$/i.test(teamId) || !kind || !id || !idPattern.test(id)) { + throw new Error("Invalid Slack workspace-qualified target"); + } + return { + kind, + id, + teamId, + raw, + normalized: `team:${teamId.toLowerCase()}:${kind}:${id.toLowerCase()}`, + }; +} + +export function formatSlackTarget(params: { + teamId?: string; + kind: SlackTargetKind; + id: string; +}): string { + const teamId = params.teamId?.trim(); + const id = params.id.trim(); + if (!teamId) { + return id; + } + const idPattern = params.kind === "user" ? /^[UW][A-Z0-9]+$/i : /^[CDG][A-Z0-9]+$/i; + if (!/^T[A-Z0-9]+$/i.test(teamId) || !idPattern.test(id)) { + throw new Error("Invalid Slack workspace-qualified target"); + } + return `team:${encodeURIComponent(teamId)}:${params.kind}:${encodeURIComponent(id)}`; +} function isUnambiguousSlackUserId(rawId: string): boolean { const id = rawId.trim(); @@ -47,6 +101,10 @@ export function parseSlackTarget( if (!trimmed) { return undefined; } + const qualifiedTarget = parseQualifiedSlackTarget(trimmed); + if (qualifiedTarget) { + return qualifiedTarget; + } const userTarget = parseMentionPrefixOrAtUserTarget({ raw: trimmed, mentionPattern: /^<@([A-Z0-9]+)>$/i, @@ -109,6 +167,9 @@ export function looksLikeSlackTargetId(raw: string): boolean { if (/^slack:/i.test(trimmed)) { return true; } + if (/^team:/i.test(trimmed)) { + return true; + } if (/^[@#]/.test(trimmed)) { return true; } diff --git a/extensions/slack/src/targets.test.ts b/extensions/slack/src/targets.test.ts index de4f04d91bfd..50d269b2114a 100644 --- a/extensions/slack/src/targets.test.ts +++ b/extensions/slack/src/targets.test.ts @@ -1,6 +1,10 @@ // Slack tests cover targets plugin behavior. import { describe, expect, it } from "vitest"; -import { canonicalizeSlackApiTargetId, slackTargetsMatch } from "./target-parsing.js"; +import { + canonicalizeSlackApiTargetId, + formatSlackTarget, + slackTargetsMatch, +} from "./target-parsing.js"; import { normalizeSlackMessagingTarget, parseSlackTarget, @@ -46,6 +50,33 @@ describe("parseSlackTarget", () => { } }); + it("parses workspace-qualified Slack targets", () => { + expect(parseSlackTarget("team:T123:channel:C456")).toEqual({ + kind: "channel", + id: "C456", + teamId: "T123", + raw: "team:T123:channel:C456", + normalized: "team:t123:channel:c456", + }); + expect(parseSlackTarget("team:T789:user:U012")).toEqual({ + kind: "user", + id: "U012", + teamId: "T789", + raw: "team:T789:user:U012", + normalized: "team:t789:user:u012", + }); + }); + + it("formats bare and structurally valid workspace-qualified targets", () => { + expect(formatSlackTarget({ teamId: "T123", kind: "channel", id: "C456" })).toBe( + "team:T123:channel:C456", + ); + expect(formatSlackTarget({ kind: "channel", id: "C456" })).toBe("C456"); + expect(() => formatSlackTarget({ teamId: "E123", kind: "channel", id: "C456" })).toThrow( + "Invalid Slack workspace-qualified target", + ); + }); + it("rejects invalid @ and # targets", () => { const cases = [ { input: "@bob-1", expectedMessage: /Slack DMs require a user id/ }, @@ -115,6 +146,11 @@ describe("slackTargetsMatch", () => { it("does not match different target kinds", () => { expect(slackTargetsMatch("user:U123", "channel:U123")).toBe(false); }); + + it("keeps identically named targets in different workspaces distinct", () => { + expect(slackTargetsMatch("team:T1:channel:C123", "team:T2:channel:C123")).toBe(false); + expect(slackTargetsMatch("team:T1:channel:C123", "channel:C123")).toBe(false); + }); }); describe("slackContextTargetsMatch", () => { diff --git a/extensions/slack/src/threading-tool-context.test.ts b/extensions/slack/src/threading-tool-context.test.ts index c3a7274761ba..b5df7bb3c820 100644 --- a/extensions/slack/src/threading-tool-context.test.ts +++ b/extensions/slack/src/threading-tool-context.test.ts @@ -312,4 +312,34 @@ describe("buildSlackThreadingToolContext", () => { expect(result.currentChannelId).toBe("user:U8SUVSVGS"); expect(result.currentMessagingTarget).toBe("user:U8SUVSVGS"); }); + + it("keeps an Enterprise channel target workspace-qualified", () => { + const result = buildSlackThreadingToolContext({ + cfg: emptyCfg, + accountId: null, + context: { + ChatType: "channel", + To: "team:T123:channel:C1234ABC", + NativeChannelId: "C1234ABC", + }, + }); + + expect(result.currentChannelId).toBe("team:T123:channel:C1234ABC"); + expect(result.currentMessagingTarget).toBe("team:T123:channel:C1234ABC"); + }); + + it("uses the physical Enterprise DM channel without losing its workspace", () => { + const result = buildSlackThreadingToolContext({ + cfg: emptyCfg, + accountId: null, + context: { + ChatType: "direct", + To: "team:T123:user:U8SUVSVGS", + NativeChannelId: "D8SRXRDNF", + }, + }); + + expect(result.currentChannelId).toBe("team:T123:channel:D8SRXRDNF"); + expect(result.currentMessagingTarget).toBe("team:T123:user:U8SUVSVGS"); + }); }); diff --git a/extensions/slack/src/threading-tool-context.ts b/extensions/slack/src/threading-tool-context.ts index 77fcb616315d..67ad74166631 100644 --- a/extensions/slack/src/threading-tool-context.ts +++ b/extensions/slack/src/threading-tool-context.ts @@ -6,6 +6,7 @@ import type { import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; import { normalizeOptionalString } from "openclaw/plugin-sdk/string-coerce-runtime"; import { resolveSlackAccount, resolveSlackReplyToMode } from "./accounts.js"; +import { formatSlackTarget, parseSlackTarget } from "./target-parsing.js"; import { normalizeSlackThreadTsCandidate } from "./thread-ts.js"; export function buildSlackThreadingToolContext(params: { @@ -32,13 +33,23 @@ export function buildSlackThreadingToolContext(params: { transportThreadTs != null || (replyToThreadTs != null && currentMessageTs != null && replyToThreadTs !== currentMessageTs); const effectiveReplyToMode = hasExplicitThreadTarget ? "all" : configuredReplyToMode; - // For channel messages, To is "channel:C…" — extract the bare ID. - // For DMs, prefer NativeChannelId for channel-scoped actions, but keep the - // user target as a valid implicit send destination when no D… id is known. const currentMessagingTarget = normalizeOptionalString(params.context.To); - const currentChannelId = currentMessagingTarget?.startsWith("channel:") - ? currentMessagingTarget.slice("channel:".length) - : (normalizeOptionalString(params.context.NativeChannelId) ?? currentMessagingTarget); + const parsedMessagingTarget = currentMessagingTarget + ? parseSlackTarget(currentMessagingTarget) + : undefined; + const nativeChannelId = normalizeOptionalString(params.context.NativeChannelId); + const currentChannelId = + parsedMessagingTarget?.teamId && nativeChannelId + ? formatSlackTarget({ + teamId: parsedMessagingTarget.teamId, + kind: "channel", + id: nativeChannelId, + }) + : parsedMessagingTarget?.teamId + ? currentMessagingTarget + : parsedMessagingTarget?.kind === "channel" + ? parsedMessagingTarget.id + : (nativeChannelId ?? currentMessagingTarget); return { currentChannelId, currentMessagingTarget,