From a94aae73b4bef0cb8fdfb9adad3afb9438005f72 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 10 May 2026 16:49:48 +0100 Subject: [PATCH] fix(slack): honor configured acp bindings Co-authored-by: Raasl <114852759+Raasl@users.noreply.github.com> --- CHANGELOG.md | 1 + docs/tools/acp-agents.md | 1 + extensions/slack/src/channel.test.ts | 60 ++++++++ extensions/slack/src/channel.ts | 55 ++++++- .../message-handler/prepare-routing.ts | 41 ++++- .../prepare.thread-session-key.test.ts | 141 +++++++++++++++++- .../src/monitor/message-handler/prepare.ts | 35 ++++- 7 files changed, 322 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 89ed8e34502..8aedb843614 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ Docs: https://docs.openclaw.ai - Slack: retain processed room messages for `requireMention=false` channels so always-on Slack rooms keep recent conversation context between turns. (#38658) Thanks @syedamaann. - Slack: compile interactive reply directives for direct outbound sends without bypassing the `interactiveReplies` capability gate, preserving Block Kit for Slack CLI and cron deliveries. (#78220) Thanks @kazamak. - Slack: keep DM last-route updates scoped to the active non-main DM session, including threaded DM turns, so isolated Slack DM sessions do not overwrite the shared main route. (#73085) Thanks @clawSean. +- Slack/ACP: route Slack channel and DM messages through configured ACP bindings when no runtime binding exists, keeping bound thread replies pinned to the persistent ACP session and dropping unavailable configured targets instead of falling back to `main`. (#73101) Thanks @Raasl. - Gateway/agents: keep structured reasons when active-run queueing fails and deprecate the legacy boolean queue helper, so steering and subagent wake diagnostics distinguish completed, non-streaming, and compacting runs. Fixes #80156. Thanks @markus-lassfolk. - Agents/UI: compact exec and tool progress rows by hiding redundant shell tool names, replacing known workspace paths with short context markers, and preserving Discord trace scrubbing for compact command lines. - ACPX: run and await the embedded ACP backend startup probe by default so the gateway `ready` signal no longer fires before the acpx runtime has either become usable or reported a probe failure; set `OPENCLAW_ACPX_RUNTIME_STARTUP_PROBE=0` to restore lazy startup. Fixes #79596. Thanks @bzelones. diff --git a/docs/tools/acp-agents.md b/docs/tools/acp-agents.md index e1743ebe0fa..032adff805a 100644 --- a/docs/tools/acp-agents.md +++ b/docs/tools/acp-agents.md @@ -335,6 +335,7 @@ top-level `bindings[]` entries. Identifies the target conversation. Per-channel shapes: - **Discord channel/thread:** `match.channel="discord"` + `match.peer.id=""` +- **Slack channel/DM:** `match.channel="slack"` + `match.peer.id="|#|userId|user:|slack:|<@userId>>"`. Prefer stable Slack ids; channel bindings also match replies inside that channel's threads. - **Telegram forum topic:** `match.channel="telegram"` + `match.peer.id=":topic:"` - **iMessage DM/group:** `match.channel="imessage"` + `match.peer.id=""`. Prefer `chat_id:*` for stable group bindings. diff --git a/extensions/slack/src/channel.test.ts b/extensions/slack/src/channel.test.ts index 1c4b2405a84..e99f927f9f5 100644 --- a/extensions/slack/src/channel.test.ts +++ b/extensions/slack/src/channel.test.ts @@ -1114,6 +1114,66 @@ describe("slackPlugin outbound new targets", () => { }); }); +describe("slackPlugin configured bindings", () => { + function requireSlackBindings() { + const bindings = slackPlugin.bindings; + if (!bindings) { + throw new Error("slack bindings adapter unavailable"); + } + return bindings; + } + + it("normalizes Slack channel and user ids for configured ACP bindings", () => { + const bindings = requireSlackBindings(); + + expect( + bindings.compileConfiguredBinding({ + binding: {} as never, + conversationId: "channel:C123", + }), + ).toEqual({ conversationId: "c123" }); + expect( + bindings.compileConfiguredBinding({ + binding: {} as never, + conversationId: "#C123", + }), + ).toEqual({ conversationId: "c123" }); + expect( + bindings.compileConfiguredBinding({ + binding: {} as never, + conversationId: "<@U123>", + }), + ).toEqual({ conversationId: "u123" }); + expect( + bindings.compileConfiguredBinding({ + binding: {} as never, + conversationId: "slack:U123", + }), + ).toEqual({ conversationId: "u123" }); + }); + + it("matches Slack thread replies against configured channel bindings", () => { + const bindings = requireSlackBindings(); + const compiledBinding = bindings.compileConfiguredBinding({ + binding: {} as never, + conversationId: "C123", + }); + + expect(compiledBinding).toEqual({ conversationId: "c123" }); + expect( + bindings.matchInboundConversation({ + binding: {} as never, + compiledBinding: compiledBinding!, + conversationId: "1770408518.451689", + parentConversationId: "C123", + }), + ).toEqual({ + conversationId: "c123", + matchPriority: 1, + }); + }); +}); + describe("slackPlugin config", () => { it("treats HTTP mode accounts with bot token + signing secret as configured", async () => { const cfg: OpenClawConfig = { diff --git a/extensions/slack/src/channel.ts b/extensions/slack/src/channel.ts index cd27f30847d..fa09cc2344c 100644 --- a/extensions/slack/src/channel.ts +++ b/extensions/slack/src/channel.ts @@ -25,7 +25,10 @@ import { createComputedAccountStatusAdapter, createDefaultChannelRuntimeState, } from "openclaw/plugin-sdk/status-helpers"; -import { normalizeOptionalString } from "openclaw/plugin-sdk/string-coerce-runtime"; +import { + normalizeLowercaseStringOrEmpty, + normalizeOptionalString, +} from "openclaw/plugin-sdk/string-coerce-runtime"; import { resolveDefaultSlackAccountId, resolveSlackAccount, @@ -240,6 +243,46 @@ function parseSlackExplicitTarget(raw: string) { }; } +function normalizeSlackAcpConversationId(raw: string | undefined | null) { + const trimmed = normalizeOptionalString(raw); + if (!trimmed) { + return null; + } + const parsed = parseSlackTarget(trimmed, { defaultKind: "channel" }); + const conversationId = normalizeLowercaseStringOrEmpty( + parsed?.id ?? trimmed.replace(/^slack:/i, "").replace(/^(?:channel|group|direct|user):/i, ""), + ); + return conversationId ? { conversationId } : null; +} + +function matchSlackAcpConversation(params: { + bindingConversationId: string; + conversationId: string; + parentConversationId?: string; +}) { + const bindingConversationId = normalizeSlackAcpConversationId( + params.bindingConversationId, + )?.conversationId; + const conversationId = normalizeSlackAcpConversationId(params.conversationId)?.conversationId; + const parentConversationId = normalizeSlackAcpConversationId( + params.parentConversationId, + )?.conversationId; + if (!bindingConversationId || !conversationId) { + return null; + } + if (bindingConversationId === conversationId) { + return { conversationId, matchPriority: 2 }; + } + if ( + parentConversationId && + parentConversationId !== conversationId && + bindingConversationId === parentConversationId + ) { + return { conversationId: parentConversationId, matchPriority: 1 }; + } + return null; +} + function buildSlackBaseSessionKey(params: { cfg: OpenClawConfig; agentId: string; @@ -520,6 +563,16 @@ export const slackPlugin: ChannelPlugin = crea resolveRequireMention: resolveSlackGroupRequireMention, resolveToolPolicy: resolveSlackGroupToolPolicy, }, + bindings: { + compileConfiguredBinding: ({ conversationId }) => + normalizeSlackAcpConversationId(conversationId), + matchInboundConversation: ({ compiledBinding, conversationId, parentConversationId }) => + matchSlackAcpConversation({ + bindingConversationId: compiledBinding.conversationId, + conversationId, + parentConversationId, + }), + }, messaging: { targetPrefixes: ["slack"], normalizeTarget: normalizeSlackMessagingTarget, diff --git a/extensions/slack/src/monitor/message-handler/prepare-routing.ts b/extensions/slack/src/monitor/message-handler/prepare-routing.ts index 0e62b9dfc76..6e3e69c7578 100644 --- a/extensions/slack/src/monitor/message-handler/prepare-routing.ts +++ b/extensions/slack/src/monitor/message-handler/prepare-routing.ts @@ -1,6 +1,8 @@ import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; import { + resolveConfiguredBindingRoute, resolveRuntimeConversationBindingRoute, + type ConfiguredBindingRouteResult, type RuntimeConversationBindingRouteResult, } from "openclaw/plugin-sdk/conversation-runtime"; import { resolveAgentRoute } from "openclaw/plugin-sdk/routing"; @@ -22,6 +24,8 @@ type SlackRoutingContext = { route: ReturnType; runtimeBinding: RuntimeConversationBindingRouteResult["bindingRecord"]; runtimeBoundSessionKey: string | undefined; + configuredBinding: ConfiguredBindingRouteResult["bindingResolution"]; + configuredBindingSessionKey: string; chatType: "direct" | "group" | "channel"; replyToMode: ReturnType; threadContext: ReturnType; @@ -232,14 +236,33 @@ export function resolveSlackRoutingContext(params: { conversationId: baseConversationId, }, }); - route = runtimeRoute.route; - const threadKeys = runtimeRoute.boundSessionKey - ? { sessionKey: route.sessionKey, parentSessionKey: undefined } - : resolveThreadSessionKeys({ - baseSessionKey: route.sessionKey, - threadId: routedThreadId, - parentSessionKey: routedThreadId && ctx.threadInheritParent ? route.sessionKey : undefined, - }); + let configuredBinding: ConfiguredBindingRouteResult["bindingResolution"] = null; + let configuredBindingSessionKey = ""; + if (runtimeRoute.boundSessionKey || runtimeRoute.bindingRecord) { + route = runtimeRoute.route; + } else { + const configuredRoute = resolveConfiguredBindingRoute({ + cfg: ctx.cfg, + route, + conversation: { + channel: "slack", + accountId: account.accountId, + conversationId: baseConversationId, + }, + }); + configuredBinding = configuredRoute.bindingResolution; + configuredBindingSessionKey = configuredRoute.boundSessionKey ?? ""; + route = configuredRoute.route; + } + const threadKeys = + runtimeRoute.boundSessionKey || configuredBindingSessionKey + ? { sessionKey: route.sessionKey, parentSessionKey: undefined } + : resolveThreadSessionKeys({ + baseSessionKey: route.sessionKey, + threadId: routedThreadId, + parentSessionKey: + routedThreadId && ctx.threadInheritParent ? route.sessionKey : undefined, + }); const sessionKey = threadKeys.sessionKey; const historyKey = isThreadReply && ctx.threadHistoryScope === "thread" ? sessionKey : message.channel; @@ -248,6 +271,8 @@ export function resolveSlackRoutingContext(params: { route, runtimeBinding: runtimeRoute.bindingRecord, runtimeBoundSessionKey: runtimeRoute.boundSessionKey, + configuredBinding, + configuredBindingSessionKey, chatType, replyToMode, threadContext, 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 3fb7baa7a2e..f3b5abc2691 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 @@ -1,5 +1,19 @@ import type { OpenClawConfig } from "openclaw/plugin-sdk/config-contracts"; -import { describe, expect, it } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const resolveConfiguredBindingRouteMock = vi.hoisted(() => vi.fn()); + +vi.mock("openclaw/plugin-sdk/conversation-runtime", async () => { + const actual = await vi.importActual( + "openclaw/plugin-sdk/conversation-runtime", + ); + return { + ...actual, + resolveConfiguredBindingRoute: (...args: unknown[]) => + resolveConfiguredBindingRouteMock(...args), + }; +}); + import type { ResolvedSlackAccount } from "../../accounts.js"; import type { SlackMessageEvent } from "../../types.js"; import { resolveSlackRoutingContext, type SlackRoutingContextDeps } from "./prepare-routing.js"; @@ -46,6 +60,131 @@ function buildChannelMessage(overrides?: Partial): SlackMessa } describe("thread-level session keys", () => { + beforeEach(() => { + resolveConfiguredBindingRouteMock.mockReset(); + resolveConfiguredBindingRouteMock.mockImplementation(({ route }) => ({ + bindingResolution: null, + route, + })); + }); + + it("routes configured ACP bindings for top-level Slack channels", () => { + const ctx = buildCtx({ replyToMode: "off" }); + const account = buildAccount("off"); + const targetSessionKey = "agent:codex:acp:binding:slack:default:c123"; + resolveConfiguredBindingRouteMock.mockImplementation(({ route, conversation }) => ({ + bindingResolution: { + conversation, + record: { + bindingId: "config:acp:slack:default:c123", + targetSessionKey, + targetKind: "session", + conversation: { + channel: "slack", + accountId: "default", + conversationId: "c123", + }, + status: "active", + boundAt: 0, + metadata: { + source: "config", + mode: "persistent", + agentId: "codex", + }, + }, + }, + boundSessionKey: targetSessionKey, + boundAgentId: "codex", + route: { + ...route, + agentId: "codex", + sessionKey: targetSessionKey, + mainSessionKey: "agent:codex:main", + matchedBy: "binding.channel", + lastRoutePolicy: "session", + }, + })); + + const routing = resolveSlackRoutingContext({ + ctx, + account, + message: buildChannelMessage({ channel: "C123" }), + isDirectMessage: false, + isGroupDm: false, + isRoom: true, + isRoomish: true, + }); + + expect(resolveConfiguredBindingRouteMock).toHaveBeenCalledWith( + expect.objectContaining({ + conversation: { + channel: "slack", + accountId: "default", + conversationId: "C123", + }, + }), + ); + expect(routing.route.agentId).toBe("codex"); + expect(routing.sessionKey).toBe(targetSessionKey); + expect(routing.configuredBindingSessionKey).toBe(targetSessionKey); + expect(routing.runtimeBinding).toBeNull(); + }); + + it("does not append Slack thread suffixes to configured ACP binding sessions", () => { + const ctx = buildCtx({ replyToMode: "all" }); + const account = buildAccount("all"); + const targetSessionKey = "agent:codex:acp:binding:slack:default:c123"; + resolveConfiguredBindingRouteMock.mockImplementation(({ route, conversation }) => ({ + bindingResolution: { + conversation, + record: { + bindingId: "config:acp:slack:default:c123", + targetSessionKey, + targetKind: "session", + conversation: { + channel: "slack", + accountId: "default", + conversationId: "c123", + }, + status: "active", + boundAt: 0, + metadata: { + source: "config", + mode: "persistent", + agentId: "codex", + }, + }, + }, + boundSessionKey: targetSessionKey, + boundAgentId: "codex", + route: { + ...route, + agentId: "codex", + sessionKey: targetSessionKey, + mainSessionKey: "agent:codex:main", + matchedBy: "binding.channel", + lastRoutePolicy: "session", + }, + })); + + const routing = resolveSlackRoutingContext({ + ctx, + account, + message: buildChannelMessage({ + channel: "C123", + ts: "1770408522.168859", + thread_ts: "1770408518.451689", + }), + isDirectMessage: false, + isGroupDm: false, + isRoom: true, + isRoomish: true, + }); + + expect(routing.sessionKey).toBe(targetSessionKey); + expect(routing.sessionKey).not.toContain(":thread:"); + }); + it("keeps top-level channel turns in one session when replyToMode=off", () => { const ctx = buildCtx({ replyToMode: "off" }); const account = buildAccount("off"); diff --git a/extensions/slack/src/monitor/message-handler/prepare.ts b/extensions/slack/src/monitor/message-handler/prepare.ts index 90db0f6c0ef..8340ad93040 100644 --- a/extensions/slack/src/monitor/message-handler/prepare.ts +++ b/extensions/slack/src/monitor/message-handler/prepare.ts @@ -14,6 +14,7 @@ import { import { resolveChannelMessageSourceReplyDeliveryMode } from "openclaw/plugin-sdk/channel-message"; import { hasControlCommand } from "openclaw/plugin-sdk/command-detection"; import { shouldHandleTextCommands } from "openclaw/plugin-sdk/command-surface"; +import { ensureConfiguredBindingRouteReady } from "openclaw/plugin-sdk/conversation-runtime"; import { formatErrorMessage } from "openclaw/plugin-sdk/error-runtime"; import { finalizeInboundContext } from "openclaw/plugin-sdk/reply-dispatch-runtime"; import { @@ -471,7 +472,9 @@ export async function prepareSlackMessage(params: { })); let mentionRegexes = resolveCachedMentionRegexes(ctx, routing.route.agentId); let wasMentioned = resolveWasMentioned(mentionRegexes); - const hasRuntimeBoundSession = Boolean(routing.runtimeBoundSessionKey); + const hasBoundSession = Boolean( + routing.runtimeBoundSessionKey || routing.configuredBindingSessionKey, + ); // Runtime bindings already pin the root and later thread replies to the same // target session, so only unbound regex mentions need a seeded thread reroute. if ( @@ -479,7 +482,7 @@ export async function prepareSlackMessage(params: { wasMentioned && isRoom && !routing.isThreadReply && - !hasRuntimeBoundSession + !hasBoundSession ) { routing = resolveSlackRoutingContext({ ctx, @@ -497,6 +500,8 @@ export async function prepareSlackMessage(params: { const { route, runtimeBinding, + configuredBinding, + configuredBindingSessionKey, replyToMode, threadContext, threadTs, @@ -510,6 +515,32 @@ export async function prepareSlackMessage(params: { `slack: routed via bound conversation ${runtimeBinding.conversation.conversationId} -> ${runtimeBinding.targetSessionKey}`, ); } + if (configuredBinding) { + const ensured = await ensureConfiguredBindingRouteReady({ + cfg, + bindingResolution: configuredBinding, + }); + if (ensured.ok) { + if (shouldLogVerbose()) { + logVerbose( + `slack: using configured ACP binding for ${configuredBinding.record.conversation.conversationId} -> ${configuredBindingSessionKey}`, + ); + } + } else { + if (shouldLogVerbose()) { + logVerbose( + `slack: configured ACP binding unavailable for ${configuredBinding.record.conversation.conversationId}: ${ensured.error}`, + ); + } + logInboundDrop({ + log: logVerbose, + channel: "slack", + reason: "configured ACP binding unavailable", + target: configuredBinding.record.conversation.conversationId, + }); + return null; + } + } let implicitMentionKinds: ReturnType = []; if ( !isDirectMessage &&